bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Philipp Stephani]

$ emacs -Q -batch -eval '(progn (defun foo () (error "Boo")) (foo))'
Boo

Emacs should print a backtrace here to make it possible to debug such
errors.  It's possible to force Emacs to print a backtrace by binding
`debug-on-error', however, that is incorrect because it changes the
meaning of `with-demoted-errors' and `condition-case-unless-debug',
causing code that would normally work to fail with an error.  Instead,
`signal' should print a backtrace if in batch mode and if it jumps to
the top-level error handler.  It looks like this logic could be
special-cased in signal_or_quit, similar to the debugger logic.


In GNU Emacs 28.0.50 (build 138, x86_64-pc-linux-gnu, GTK+ Version 3.24.22, cairo version 1.16.0)
 of 2020-11-28
Repository revision: a7825c4be06b7c0b544df34555ecf586276245e6
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux rodete

Configured using:
 'configure --enable-gcc-warnings=warn-only
 --enable-gtk-deprecation-warnings --without-pop --with-mailutils
 --enable-checking=all --enable-check-lisp-object-type --with-modules
 'CFLAGS=-O0 -ggdb3''

Configured features:
XPM JPEG TIFF GIF PNG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY INOTIFY
LIBSELINUX GNUTLS FREETYPE HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS GTK3 X11
XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER

Important settings:
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc dired dired-loaddefs rfc822
mml easymenu mml-sec epa epg epg-config gnus-util rmail rmail-loaddefs
time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils phst skeleton derived edmacro kmacro pcase ffap
thingatpt url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf url-util url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache json map url-vars
mailcap rx gnutls puny dbus xml subr-x seq byte-opt gv bytecomp
byte-compile cconv compile text-property-search comint ansi-color ring
cl-loaddefs cl-lib tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 69693 6335)
 (symbols 48 8576 1)
 (strings 32 24393 1275)
 (string-bytes 1 790267)
 (vectors 16 13891)
 (vector-slots 8 188843 5960)
 (floats 8 26 30)
 (intervals 56 212 0)
 (buffers 984 11))

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich.  Falls Sie diese fälschlicherweise erhalten haben
sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie
alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail
an die falsche Person gesendet wurde.

This e-mail is confidential.  If you received this communication by mistake,
please don’t forward it to anyone else, please erase all copies and
attachments, and please let me know that it has gone to the wrong person.

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

Am So., 29. Nov. 2020 um 11:48 Uhr schrieb Philipp Stephani
<p.stephani2@gmail.com>:
>
>
> $ emacs -Q -batch -eval '(progn (defun foo () (error "Boo")) (foo))'
> Boo
>
> Emacs should print a backtrace here to make it possible to debug such
> errors.  It's possible to force Emacs to print a backtrace by binding
> `debug-on-error', however, that is incorrect because it changes the
> meaning of `with-demoted-errors' and `condition-case-unless-debug',
> causing code that would normally work to fail with an error.  Instead,
> `signal' should print a backtrace if in batch mode and if it jumps to
> the top-level error handler.  It looks like this logic could be
> special-cased in signal_or_quit, similar to the debugger logic.
>


I've attached a patch.

[-- Attachment #2: 0001-Print-a-backtrace-on-unhandled-errors-in-batch-mode-.patch --]
[-- Type: text/x-patch, Size: 4861 bytes --]

From 6441ca2ca9ed0233cdf474cc64a860a3cff51222 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sun, 29 Nov 2020 14:24:57 +0100
Subject: [PATCH] Print a backtrace on unhandled errors in batch mode
 (Bug#44942).

* src/eval.c (signal_or_quit): Print a backtrace in batch mode if no
error handler was found.

* test/src/eval-tests.el (eval-tests/backtrace-in-batch-mode)
(eval-tests/backtrace-in-batch-mode/demoted-errors): New unit tests.

* etc/NEWS: Document change.
---
 etc/NEWS               |  3 +++
 src/eval.c             | 15 ++++++++++++++-
 test/src/eval-tests.el | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 2fb33e342e..baae624f27 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -192,6 +192,9 @@ have been replaced with "chat.freenode.net" throughout Emacs.
 These functions return the connection local value of the respective
 variables.  This can be used for remote hosts.
 
+** Emacs now prints a backtrace when signaling an error in batch mode.  This
+makes debugging Emacs Lisp scripts run in batch mode easier.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/src/eval.c b/src/eval.c
index d9a424b57a..18df484aac 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1709,6 +1709,7 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
 	break;
     }
 
+  bool debugger_called = false;
   if (/* Don't run the debugger for a memory-full error.
 	 (There is no room in memory to do that!)  */
       !NILP (error_symbol)
@@ -1722,7 +1723,7 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
 	     if requested".  */
 	  || EQ (h->tag_or_ch, Qerror)))
     {
-      bool debugger_called
+      debugger_called
 	= maybe_call_debugger (conditions, error_symbol, data);
       /* We can't return values to code which signaled an error, but we
 	 can continue code which has signaled a quit.  */
@@ -1730,6 +1731,18 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
 	return Qnil;
     }
 
+  /* If we're in batch mode, print a backtrace unconditionally to help with
+     debugging.  Make sure to use `debug' unconditionally to not interfere with
+     ERT or other packages that install custom debuggers.  */
+  if (!debugger_called && !NILP (error_symbol)
+      && (NILP (clause) || EQ (h->tag_or_ch, Qerror)) && noninteractive)
+    {
+      ptrdiff_t count = SPECPDL_INDEX ();
+      specbind (Vdebugger, Qdebug);
+      call_debugger (list2 (Qerror, Fcons (error_symbol, data)));
+      unbind_to (count, Qnil);
+    }
+
   if (!NILP (clause))
     {
       Lisp_Object unwind_data
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index 074f5be1ef..4125573dc6 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -27,6 +27,7 @@
 
 (require 'ert)
 (eval-when-compile (require 'cl-lib))
+(require 'subr-x)
 
 (ert-deftest eval-tests--bug24673 ()
   "Check that Bug#24673 has been fixed."
@@ -176,4 +177,36 @@ eval-tests-19790-backquote-comma-dot-substitution
 expressions works for identifiers starting with period."
   (should (equal (let ((.x 'identity)) (eval `(,.x 'ok))) 'ok)))
 
+(ert-deftest eval-tests/backtrace-in-batch-mode ()
+  (let ((emacs (expand-file-name invocation-name invocation-directory)))
+    (skip-unless (file-executable-p emacs))
+    (with-temp-buffer
+      (let ((status (call-process emacs nil t nil
+                                  "--quick" "--batch"
+                                  (concat "--eval="
+                                          (prin1-to-string
+                                           '(progn
+                                              (defun foo () (error "Boo"))
+                                              (foo)))))))
+        (should (natnump status))
+        (should-not (eql status 0)))
+      (goto-char (point-min))
+      (ert-info ((concat "Process output:\n" (buffer-string)))
+        (search-forward "  foo()")
+        (search-forward "  normal-top-level()")))))
+
+(ert-deftest eval-tests/backtrace-in-batch-mode/demoted-errors ()
+  (let ((emacs (expand-file-name invocation-name invocation-directory)))
+    (skip-unless (file-executable-p emacs))
+    (with-temp-buffer
+      (should (eql  0 (call-process emacs nil t nil
+                                    "--quick" "--batch"
+                                    (concat "--eval="
+                                            (prin1-to-string
+                                             '(with-demoted-errors "Error: %S"
+                                                (error "Boo")))))))
+      (goto-char (point-min))
+      (should (equal (string-trim (buffer-string))
+                     "Error: (error \"Boo\")")))))
+
 ;;; eval-tests.el ends here
-- 
2.29.2.454.gaff20da3a2-goog

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Philipp Stephani]

Am So., 29. Nov. 2020 um 16:27 Uhr schrieb Philipp Stephani
<p.stephani2@gmail.com>:
>
> Am So., 29. Nov. 2020 um 11:48 Uhr schrieb Philipp Stephani
> <p.stephani2@gmail.com>:
> >
> >
> > $ emacs -Q -batch -eval '(progn (defun foo () (error "Boo")) (foo))'
> > Boo
> >
> > Emacs should print a backtrace here to make it possible to debug such
> > errors.  It's possible to force Emacs to print a backtrace by binding
> > `debug-on-error', however, that is incorrect because it changes the
> > meaning of `with-demoted-errors' and `condition-case-unless-debug',
> > causing code that would normally work to fail with an error.  Instead,
> > `signal' should print a backtrace if in batch mode and if it jumps to
> > the top-level error handler.  It looks like this logic could be
> > special-cased in signal_or_quit, similar to the debugger logic.
> >
>
>
> I've attached a patch.

Pushed to master as commit 40e11743ca.

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 6 Dec 2020 17:50:17 +0100
> 
> Am So., 29. Nov. 2020 um 16:27 Uhr schrieb Philipp Stephani
> <p.stephani2@gmail.com>:
> >
> > Am So., 29. Nov. 2020 um 11:48 Uhr schrieb Philipp Stephani
> > <p.stephani2@gmail.com>:
> > >
> > >
> > > $ emacs -Q -batch -eval '(progn (defun foo () (error "Boo")) (foo))'
> > > Boo
> > >
> > > Emacs should print a backtrace here to make it possible to debug such
> > > errors.  It's possible to force Emacs to print a backtrace by binding
> > > `debug-on-error', however, that is incorrect because it changes the
> > > meaning of `with-demoted-errors' and `condition-case-unless-debug',
> > > causing code that would normally work to fail with an error.  Instead,
> > > `signal' should print a backtrace if in batch mode and if it jumps to
> > > the top-level error handler.  It looks like this logic could be
> > > special-cased in signal_or_quit, similar to the debugger logic.
> > >
> >
> > I've attached a patch.
> 
> Pushed to master as commit 40e11743ca.

This is an incompatible change.  Is there a way to get back previous
behavior?  If so, please mention that in NEWS; if not, let's please
provide a way to get back the old behavior.

Thanks.

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Philipp Stephani]

Am So., 6. Dez. 2020 um 18:00 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sun, 6 Dec 2020 17:50:17 +0100
> >
> > Am So., 29. Nov. 2020 um 16:27 Uhr schrieb Philipp Stephani
> > <p.stephani2@gmail.com>:
> > >
> > > Am So., 29. Nov. 2020 um 11:48 Uhr schrieb Philipp Stephani
> > > <p.stephani2@gmail.com>:
> > > >
> > > >
> > > > $ emacs -Q -batch -eval '(progn (defun foo () (error "Boo")) (foo))'
> > > > Boo
> > > >
> > > > Emacs should print a backtrace here to make it possible to debug such
> > > > errors.  It's possible to force Emacs to print a backtrace by binding
> > > > `debug-on-error', however, that is incorrect because it changes the
> > > > meaning of `with-demoted-errors' and `condition-case-unless-debug',
> > > > causing code that would normally work to fail with an error.  Instead,
> > > > `signal' should print a backtrace if in batch mode and if it jumps to
> > > > the top-level error handler.  It looks like this logic could be
> > > > special-cased in signal_or_quit, similar to the debugger logic.
> > > >
> > >
> > > I've attached a patch.
> >
> > Pushed to master as commit 40e11743ca.
>
> This is an incompatible change.

Is it? In other words, do we guarantee that no stack trace is being
printed if debug-on-error is nil? I'd say that parsing error messages
is brittle anyway, and we're not obliged to provide stable output.

>  Is there a way to get back previous
> behavior?  If so, please mention that in NEWS; if not, let's please
> provide a way to get back the old behavior.

I'd be OK adding a variable for this, but on the other hand I'm not
really convinced that this is really a breaking change (see above).

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Eli Zaretskii]

On December 7, 2020 12:29:16 PM GMT+02:00, Philipp Stephani <p.stephani2@gmail.com> wrote:
> Am So., 6. Dez. 2020 um 18:00 Uhr schrieb Eli Zaretskii
> <eliz@gnu.org>:
> >
> > > From: Philipp Stephani <p.stephani2@gmail.com>
> > > Date: Sun, 6 Dec 2020 17:50:17 +0100
> > >
> > > Am So., 29. Nov. 2020 um 16:27 Uhr schrieb Philipp Stephani
> > > <p.stephani2@gmail.com>:
> > > >
> > > Pushed to master as commit 40e11743ca.
> >
> > This is an incompatible change.
> 
> Is it? In other words, do we guarantee that no stack trace is being
> printed if debug-on-error is nil? I'd say that parsing error messages
> is brittle anyway, and we're not obliged to provide stable output

The behavior did change.  Since Emacs doesn't have a formal spec  its long-time behavior is a de-facto standard we should strive hard not to break.  As Stefan just pointed out, this change already broke 2 tests  if we need any proof.

> >  Is there a way to get back previous
> > behavior?  If so, please mention that in NEWS; if not, let's please
> > provide a way to get back the old behavior.
> 
> I'd be OK adding a variable for this, but on the other hand I'm not
> really convinced that this is really a breaking change (see above).

Yes, we do need to be able to get back the old bdhavior.

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Philipp Stephani]

Am Mo., 7. Dez. 2020 um 11:42 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> On December 7, 2020 12:29:16 PM GMT+02:00, Philipp Stephani <p.stephani2@gmail.com> wrote:
> > Am So., 6. Dez. 2020 um 18:00 Uhr schrieb Eli Zaretskii
> > <eliz@gnu.org>:
> > >
> > > > From: Philipp Stephani <p.stephani2@gmail.com>
> > > > Date: Sun, 6 Dec 2020 17:50:17 +0100
> > > >
> > > > Am So., 29. Nov. 2020 um 16:27 Uhr schrieb Philipp Stephani
> > > > <p.stephani2@gmail.com>:
> > > > >
> > > > Pushed to master as commit 40e11743ca.
> > >
> > > This is an incompatible change.
> >
> > Is it? In other words, do we guarantee that no stack trace is being
> > printed if debug-on-error is nil? I'd say that parsing error messages
> > is brittle anyway, and we're not obliged to provide stable output
>
> The behavior did change.  Since Emacs doesn't have a formal spec  its long-time behavior is a de-facto standard we should strive hard not to break.  As Stefan just pointed out, this change already broke 2 tests  if we need any proof.
>
> > >  Is there a way to get back previous
> > > behavior?  If so, please mention that in NEWS; if not, let's please
> > > provide a way to get back the old behavior.
> >
> > I'd be OK adding a variable for this, but on the other hand I'm not
> > really convinced that this is really a breaking change (see above).
>
> Yes, we do need to be able to get back the old bdhavior.

Fair enough, I've added a new boolean variable for this.

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Mattias Engdegård]

This is a fine and welcome improvement, thank you! Perhaps it needn't say 'Debugger entered' though? It's a bit misleading and not really true from the user's point of view.

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Philipp Stephani]

Am Mo., 7. Dez. 2020 um 16:07 Uhr schrieb Mattias Engdegård <mattiase@acm.org>:
>
> This is a fine and welcome improvement, thank you! Perhaps it needn't say 'Debugger entered' though? It's a bit misleading and not really true from the user's point of view.
>

Yes, that would be great :-)
Unfortunately I haven't found a trivial way to separate the
backtrace-printing that `debug' does from its other actions, but I'm
planning to take a second look at the debug.el code.

bug#44942: 28.0.50; Emacs should print a backtrace on uncaught errors in batch mode

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 7 Dec 2020 12:07:48 +0100
> Cc: 44942@debbugs.gnu.org
> 
> > Yes, we do need to be able to get back the old bdhavior.
> 
> Fair enough, I've added a new boolean variable for this.

Thank you.