bug: notmuch-poll doesn't check return value

bug: notmuch-poll doesn't check return value

[Ketil Malde]

Hi,

I've been stumped occasionally when new mail fails to appear in notmuch,
and then I suddenly discover it by some other means (web front end,
mobile, etc).

The problem is that notmuch-poll fails silently when it fails, which is
usually due to broken symlinks in my mail directories, or some IMAP
configuration mistake.  Easy to fix when you know about it.

I've, uh, "fixed" this by redefining notmuch-poll like so:

---8<-------------------------------------
;;; Make notmuch-poll fail gracelessly when something goes wrong
;;; Better than failing silently, in ancy case.
(defun notmuch-poll ()
  "Run \"notmuch new\" or an external script to import mail.

Invokes `notmuch-poll-script', \"notmuch new\", or does nothing
depending on the value of `notmuch-poll-script'."
  (interactive)
  (if (not (equal 0
		  (if (stringp notmuch-poll-script)
		      (unless (string= notmuch-poll-script "")
			(call-process notmuch-poll-script nil nil))
		    (call-process notmuch-command nil nil nil "new"))))
     (error "Notmuch: poll failed!")))
---8<-------------------------------------

Surely, somebody who knows lisp can do something far more elegant than
this, but the point is to bug out when something doesn't work.

In addtion, I had to fix my notmuch-poll-script, which contains
'offlineimap', then 'notmuch new', followed by some tagging commands.
As the tagging commands tend to work even if 'new' doesn't, this will
often (pretend to) exit successfully.  A 'set -e' at the top of the
script is probably a good idea, and should probably be mentioned in
documentation somewhere.

-k
-- 
If I haven't seen further, it is by standing in the footprints of giants

[PATCH] emacs: poll: return useful errors when poll fails.

[Mark Walters]

Previously poll called from emacs would fail silently. This makes it
return a useful error message.

In the non-deprecated case of notmuch new and appropriate hooks, it
uses notmuch-call-notmuch-process which gives an error and
additionally puts the stdout/stderr etc in the *Notmuch errors*
buffer.

In the deprecated case of a custom poll script it only returns an
error message.

Commit based on a bug report, and a potential fix, by Ketil Malde.
---

This should fix the bug reported in the parent message -- we should
definitely report error messages. It might be nice to output the
stdout/stderr in the custom poll script case but since we don't have
built in infrastructure for it it probably isn't worth it for a
deprecated case.

(Note I don't use poll, so this is not heavily tested)

Best wishes

Mark



 emacs/notmuch-lib.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 1c3a9fe..89c01a5 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -243,8 +243,9 @@ depending on the value of `notmuch-poll-script'."
   (interactive)
   (if (stringp notmuch-poll-script)
       (unless (string= notmuch-poll-script "")
-	(call-process notmuch-poll-script nil nil))
-    (call-process notmuch-command nil nil nil "new")))
+	(unless (equal (call-process notmuch-poll-script nil nil) 0)
+	  (error "Notmuch: poll script `%s' failed!" notmuch-poll-script)))
+    (notmuch-call-notmuch-process "new")))
 
 (defun notmuch-bury-or-kill-this-buffer ()
   "Undisplay the current buffer.
-- 
2.1.4

Re: [PATCH] emacs: poll: return useful errors when poll fails.

[Tomi Ollila]

On Wed, Oct 28 2015, Mark Walters <markwalters1009@gmail.com> wrote:

> Previously poll called from emacs would fail silently. This makes it
> return a useful error message.
>
> In the non-deprecated case of notmuch new and appropriate hooks, it
> uses notmuch-call-notmuch-process which gives an error and
> additionally puts the stdout/stderr etc in the *Notmuch errors*
> buffer.
>
> In the deprecated case of a custom poll script it only returns an
> error message.
>
> Commit based on a bug report, and a potential fix, by Ketil Malde.
> ---

Looks good (to me) -- I investigated a bit how notmuch-call-notmuch-process
does things; it uses notmuch-check-exit-status which is not applicable 
here.

Mark said he doesn't use poll -- and I have rewritten my poll(*) so my
tests will be as lightweight as his. So we wait for Ketil's report
how this works :D

Tomi

(*) see at the end of this email

>
> This should fix the bug reported in the parent message -- we should
> definitely report error messages. It might be nice to output the
> stdout/stderr in the custom poll script case but since we don't have
> built in infrastructure for it it probably isn't worth it for a
> deprecated case.
>
> (Note I don't use poll, so this is not heavily tested)
>
> Best wishes
>
> Mark
>

For reference, if anyone gets ideas from this...

;; overwrite notmuch-poll defined in notmuch.el
(defun notmuch-poll ()
  (interactive)
  (save-window-excursion
    (let ((buffer (get-buffer-create "*my notmuch polls*")))
      (set-buffer buffer)
      (setq buffer-read-only nil)
      (goto-char (point-max))
      (pop-to-buffer buffer)
      (call-process "notmuch-log-output.sh" nil t t "new" "--verbose")
      (bury-buffer))) ;; <- b-b to put the buffer at the "bottom" of buffers
  (message "update finished"))

Re: [PATCH] emacs: poll: return useful errors when poll fails.

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Previously poll called from emacs would fail silently. This makes it
> return a useful error message.
>
> In the non-deprecated case of notmuch new and appropriate hooks, it
> uses notmuch-call-notmuch-process which gives an error and
> additionally puts the stdout/stderr etc in the *Notmuch errors*
> buffer.
>
> In the deprecated case of a custom poll script it only returns an
> error message.
>
> Commit based on a bug report, and a potential fix, by Ketil Malde.

applied to release and master

d