GNU bug report logs - #40016
[PATCH] inferior: Distinguish inferior exceptions.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 40016 in the body.
You can then email your comments to 40016 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Christopher Baines <mail <at> cbaines.net>
Subject: [PATCH] inferior: Distinguish inferior exceptions.
Date: Tue, 10 Mar 2020 17:03:05 +0100

This avoids ambiguities when looking at a backtrace where the exception
was actually thrown by an inferior in a very different context.

* guix/inferior.scm (&inferior-exception): New condition type.
(read-repl-response): Add optional 'inferior' parameter.  Raise
'&inferior-exception' instead of rethrowing to KEY when receiving an
'exception' message.
(read-inferior-response): Pass INFERIOR to 'read-repl-response'.
* tests/inferior.scm ("&inferior-exception"): New test.
---
 guix/inferior.scm  | 21 +++++++++++++++++----
 tests/inferior.scm | 13 ++++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/guix/inferior.scm b/guix/inferior.scm
index 0236fb61ad..6b685ece30 100644
--- a/guix/inferior.scm
+++ b/guix/inferior.scm
@@ -63,6 +63,9 @@
             inferior-eval
             inferior-eval-with-store
             inferior-object?
+            inferior-exception?
+            inferior-exception-arguments
+            inferior-exception-inferior
             read-repl-response
 
             inferior-packages
@@ -195,8 +198,15 @@ equivalent.  Return #f if the inferior could not be launched."
 
 (set-record-type-printer! <inferior-object> write-inferior-object)
 
-(define (read-repl-response port)
-  "Read a (guix repl) response from PORT and return it as a Scheme object."
+;; Reified exception thrown by an inferior.
+(define-condition-type &inferior-exception &error
+  inferior-exception?
+  (arguments  inferior-exception-arguments)       ;key + arguments
+  (inferior   inferior-exception-inferior))       ;<inferior> | #f
+
+(define* (read-repl-response port #:optional inferior)
+  "Read a (guix repl) response from PORT and return it as a Scheme object.
+Raise '&inferior-exception' when an exception is read from PORT."
   (define sexp->object
     (match-lambda
       (('value value)
@@ -208,10 +218,13 @@ equivalent.  Return #f if the inferior could not be launched."
     (('values objects ...)
      (apply values (map sexp->object objects)))
     (('exception key objects ...)
-     (apply throw key (map sexp->object objects)))))
+     (raise (condition (&inferior-exception
+                        (arguments (cons key (map sexp->object objects)))
+                        (inferior inferior)))))))
 
 (define (read-inferior-response inferior)
-  (read-repl-response (inferior-socket inferior)))
+  (read-repl-response (inferior-socket inferior)
+                      inferior))
 
 (define (send-inferior-request exp inferior)
   (write exp (inferior-socket inferior))
diff --git a/tests/inferior.scm b/tests/inferior.scm
index f54b6d6037..b4417d8629 100644
--- a/tests/inferior.scm
+++ b/tests/inferior.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2018, 2019 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2018, 2019, 2020 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -61,6 +61,17 @@
            (close-inferior inferior)
            (list a (inferior-object? b))))))
 
+(test-equal "&inferior-exception"
+  '(a b c d)
+  (let ((inferior (open-inferior %top-builddir
+                                 #:command "scripts/guix")))
+    (guard (c ((inferior-exception? c)
+               (close-inferior inferior)
+               (and (eq? inferior (inferior-exception-inferior c))
+                    (inferior-exception-arguments c))))
+      (inferior-eval '(throw 'a 'b 'c 'd) inferior)
+      'badness)))
+
 (test-equal "inferior-packages"
   (take (sort (fold-packages (lambda (package lst)
                                (cons (list (package-name package)
-- 
2.25.1

Message #8 received at 40016 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 40016 <at> debbugs.gnu.org
Cc: Christopher Baines <mail <at> cbaines.net>
Subject: Re: [bug#40016] [PATCH] inferior: Distinguish inferior exceptions.
Date: Tue, 10 Mar 2020 18:30:48 +0100

Hello,

Ludovic Courtès <ludo <at> gnu.org> skribis:

> This avoids ambiguities when looking at a backtrace where the exception
> was actually thrown by an inferior in a very different context.

The idea was suggested at the Guix Days by someone who had had a hard
time looking at one of these weird backtraces, until you realize the
exception was actually thrown by another process.

There are a few places in the Data Service that do things like:

  (catch 'misc-error
    (lambda ()
      (inferior-eval …))
    …)

This will have to be adjusted.  Does that work for you, Chris?

If you want to go fancy, you can implement a compatibility later, though
I’m not sure it’s worth it.

Eventually I’d like ‘&inferior-exception’ to include inferior stack
frames, though that’ll require changes to the REPL protocol.

Ludo’.

Message #11 received at 40016 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 40016 <at> debbugs.gnu.org
Subject: Re: [bug#40016] [PATCH] inferior: Distinguish inferior exceptions.
Date: Wed, 11 Mar 2020 00:09:29 +0000

[Message part 1 (text/plain, inline)]

Ludovic Courtès <ludo <at> gnu.org> writes:

> Ludovic Courtès <ludo <at> gnu.org> skribis:
>
>> This avoids ambiguities when looking at a backtrace where the exception
>> was actually thrown by an inferior in a very different context.
>
> The idea was suggested at the Guix Days by someone who had had a hard
> time looking at one of these weird backtraces, until you realize the
> exception was actually thrown by another process.
>
> There are a few places in the Data Service that do things like:
>
>   (catch 'misc-error
>     (lambda ()
>       (inferior-eval …))
>     …)
>
> This will have to be adjusted.  Does that work for you, Chris?

Yeah that sounds fine. Generally I think it's good to keep backwards
compatibility with the Guix Data Service, so I'd be looking to support
both ways exceptions can be raised, but that doesn't seem to difficult.

Thanks,

Chris

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 40016 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 40016 <at> debbugs.gnu.org
Subject: Re: [bug#40016] [PATCH] inferior: Distinguish inferior exceptions.
Date: Wed, 11 Mar 2020 14:54:22 +0100

Hi!

Christopher Baines <mail <at> cbaines.net> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Ludovic Courtès <ludo <at> gnu.org> skribis:
>>
>>> This avoids ambiguities when looking at a backtrace where the exception
>>> was actually thrown by an inferior in a very different context.
>>
>> The idea was suggested at the Guix Days by someone who had had a hard
>> time looking at one of these weird backtraces, until you realize the
>> exception was actually thrown by another process.
>>
>> There are a few places in the Data Service that do things like:
>>
>>   (catch 'misc-error
>>     (lambda ()
>>       (inferior-eval …))
>>     …)
>>
>> This will have to be adjusted.  Does that work for you, Chris?
>
> Yeah that sounds fine.

Good.

> Generally I think it's good to keep backwards compatibility with the
> Guix Data Service, so I'd be looking to support both ways exceptions
> can be raised, but that doesn't seem to difficult.

Yes.  Though you’ll also have to handle Guile 3.0 vs. 2.2 exception
types, in particular wrt. SRFI-34 exceptions (see Guix commit
7f3bbfaf8ec3b96e02e0cf74e7515ac33c002107.)

Thanks for your feedback,
Ludo’.

Message #19 received at 40016-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 40016-done <at> debbugs.gnu.org
Cc: Christopher Baines <mail <at> cbaines.net>
Subject: Re: [bug#40016] [PATCH] inferior: Distinguish inferior exceptions.
Date: Thu, 12 Mar 2020 13:43:41 +0100

Ludovic Courtès <ludo <at> gnu.org> skribis:

> This avoids ambiguities when looking at a backtrace where the exception
> was actually thrown by an inferior in a very different context.
>
> * guix/inferior.scm (&inferior-exception): New condition type.
> (read-repl-response): Add optional 'inferior' parameter.  Raise
> '&inferior-exception' instead of rethrowing to KEY when receiving an
> 'exception' message.
> (read-inferior-response): Pass INFERIOR to 'read-repl-response'.
> * tests/inferior.scm ("&inferior-exception"): New test.

Pushed as f7537e30b892cef09d91902547c00e5fa9b66f3b.

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.