[bug#40016] [PATCH] inferior: Distinguish inferior exceptions.

[bug#40016] [PATCH] inferior: Distinguish inferior exceptions.

[Ludovic Courtès]

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@gnu.org>
+;;; Copyright © 2018, 2019, 2020 Ludovic Courtès <ludo@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

[bug#40016] [PATCH] inferior: Distinguish inferior exceptions.

[Ludovic Courtès]

Hello,

Ludovic Courtès <ludo@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’.

[bug#40016] [PATCH] inferior: Distinguish inferior exceptions.

[Christopher Baines]

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


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

> Ludovic Courtès <ludo@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

[bug#40016] [PATCH] inferior: Distinguish inferior exceptions.

[Ludovic Courtès]

Hi!

Christopher Baines <mail@cbaines.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Ludovic Courtès <ludo@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’.

bug#40016: [PATCH] inferior: Distinguish inferior exceptions.

[Ludovic Courtès]

Ludovic Courtès <ludo@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’.