GNU bug report logs - #26489
[PATCH] substitute: Ignore bad responses.

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 26489 in the body.
You can then email your comments to 26489 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: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: guix-patches <at> gnu.org
Subject: [PATCH] substitute: Ignore bad responses.
Date: Fri, 14 Apr 2017 02:27:55 +0200

* guix/scripts/substitute.scm (http-multiple-get): Catch BAD-RESPONSE
exceptions and keep going.
---

Guix,

One weird HTTP response from a server will kill ‘guix substitute’:

  updating list of substitutes from 'https://foo'...  50.0%Backtrace:
  ...
  guix/ui.scm:1229:8: In procedure run-guix-command:
  guix/ui.scm:1229:8: Throw to key `bad-response' with args
                      `("Bad Response-Line: ~s" (""))'.
  error: build failed: substituter `substitute' died unexpectedly

Attached is a patch to ignore such bad responses. The offending .narinfo
will be ignored for that session, and not cached at all. The result:

  updating list of substitutes from 'https://bar'... 100.0%
  updating list of substitutes from 'https://foo'...   2.9% (bad response)
  updating list of substitutes from 'https://foo'...   5.9% (bad response)

As a nice bonus, guix doesn't keel over and die.

Is this the best solution? A good one? Should it be made more obvious
that only READ-RESPONSE can throw, and that PROC will never be called
with, a bad response? No idea. I haven't had enough free time to learn
good the Guile like I'd so hoped to do at the beginning of the year. :c

Be gentle, dear reader, and all that,

T G-R

 guix/scripts/substitute.scm | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index d3bccf4dd..7eccf9831 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -564,18 +564,24 @@ initial connection on which HTTP requests are sent."
           (()
            (reverse result))
           ((head tail ...)
-           (let* ((resp   (read-response p))
-                  (body   (response-body-port resp))
-                  (result (proc head resp body result)))
-             ;; The server can choose to stop responding at any time, in which
-             ;; case we have to try again.  Check whether that is the case.
-             ;; Note that even upon "Connection: close", we can read from BODY.
-             (match (assq 'connection (response-headers resp))
-               (('connection 'close)
-                (close-connection p)
-                (connect #f tail result))         ;try again
-               (_
-                (loop tail result))))))))))       ;keep going
+           (catch 'bad-response
+             (lambda ()
+               (let* ((resp   (read-response p))
+                      (body   (response-body-port resp))
+                      (result (proc head resp body result)))
+                 ;; The server can stop responding at any time, in which case
+                 ;; we have to try again.  Check whether that's the case.  Note
+                 ;; that we can read from BODY even upon "Connection: close".
+                 (match (assq 'connection (response-headers resp))
+                   (('connection 'close)
+                    (close-connection p)
+                    (connect #f tail result)) ; try again
+                   (_
+                    (loop tail result))))) ; keep going
+             (lambda args
+               ;; This message appears on the same line as the progress report.
+               (format (current-error-port) " (bad response)~%")
+               (loop tail result))))))))) ; keep going
 
 (define (read-to-eof port)
   "Read from PORT until EOF is reached.  The data are discarded."
-- 
2.12.2

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 26489 <at> debbugs.gnu.org
Subject: Re: bug#26489: [PATCH] substitute: Ignore bad responses.
Date: Fri, 14 Apr 2017 11:54:32 +0200

Howdy!

Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:

> * guix/scripts/substitute.scm (http-multiple-get): Catch BAD-RESPONSE
> exceptions and keep going.
> ---
>
> Guix,
>
> One weird HTTP response from a server will kill ‘guix substitute’:
>
>   updating list of substitutes from 'https://foo'...  50.0%Backtrace:
>   ...
>   guix/ui.scm:1229:8: In procedure run-guix-command:
>   guix/ui.scm:1229:8: Throw to key `bad-response' with args
>                       `("Bad Response-Line: ~s" (""))'.
>   error: build failed: substituter `substitute' died unexpectedly
>
> Attached is a patch to ignore such bad responses. The offending .narinfo
> will be ignored for that session, and not cached at all. The result:

I’m sure you expect this question: what bad responses did you get in
practice?  :-)

Usually that is a sign of a broken HTTP server.  Of course it’s
widespread enough, we’d better handle it, either in Guix or directly in
(web client) in Guile; OTOH, if it’s a genuine problem, we’d better not
hide it.

Thanks,
Ludo’.

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

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: ludo <at> gnu.org
Cc: 26489 <at> debbugs.gnu.org
Subject: Re: bug#26489: [PATCH] substitute: Ignore bad responses.
Date: Fri, 28 Apr 2017 22:56:34 +0200

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

Ludo',

I should really send this message in my Drafts folder since 14/4... :-/

On 14/04/17 11:54, Ludovic Courtès wrote:
> Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:
>> One weird HTTP response from a server will kill ‘guix substitute’:
>> 
>> updating list of substitutes from 'https://foo'... 50.0%Backtrace:
>>  ... guix/ui.scm:1229:8: In procedure run-guix-command: 
>> guix/ui.scm:1229:8: Throw to key `bad-response' with args `("Bad 
>> Response-Line: ~s" (""))'. error: build failed: substituter 
>> `substitute' died unexpectedly
>> 
>> Attached is a patch to ignore such bad responses. The offending 
>> .narinfo will be ignored for that session, and not cached at all. 
>> The result:
> 
> I’m sure you expect this question: what bad responses did you get in 
> practice?  :-)

In fact, not really. The error message looked unambiguous to me: the
HTTP response (the first line returned to the client, e.g. "HTTP/1.1 200
OK") was simply empty, throwing an exception.

Interestingly, a newline seems to be required.

Using http://bad.http.response.tobias.gr as a substitute server triggers
it. http://no.http.response.tobias.gr does not.

> Usually that is a sign of a broken HTTP server.

I think it's actually something in-between me and the server. I'll take
a closer look next time this happens.

> Of course it’s  widespread enough, we’d better handle it, either in 
> Guix or directly in (web client) in Guile;

As I read it, (web client) considers throwing a BAD-RESPONSE exception
the best or only way to deal with an error like this. I agree.

> OTOH, if it’s a genuine problem, we’d better not hide it.

Well, we don't hide it, per se. Hence the error message.

I think throwing an unhandled exception is definitely the wrong thing to
do here — this kills even ‘guix --keep-going --fallback’. I'm less sure
about the right place to do it

Kind regards,

T G-R

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

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 26489 <at> debbugs.gnu.org
Subject: Re: bug#26489: [PATCH] substitute: Ignore bad responses.
Date: Mon, 01 May 2017 15:14:44 +0200

Hi!

Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:

> On 14/04/17 11:54, Ludovic Courtès wrote:
>> Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:
>>> One weird HTTP response from a server will kill ‘guix substitute’:
>>> 
>>> updating list of substitutes from 'https://foo'... 50.0%Backtrace:
>>>  ... guix/ui.scm:1229:8: In procedure run-guix-command: 
>>> guix/ui.scm:1229:8: Throw to key `bad-response' with args `("Bad 
>>> Response-Line: ~s" (""))'. error: build failed: substituter 
>>> `substitute' died unexpectedly
>>> 
>>> Attached is a patch to ignore such bad responses. The offending 
>>> .narinfo will be ignored for that session, and not cached at all. 
>>> The result:
>> 
>> I’m sure you expect this question: what bad responses did you get in 
>> practice?  :-)
>
> In fact, not really. The error message looked unambiguous to me: the
> HTTP response (the first line returned to the client, e.g. "HTTP/1.1 200
> OK") was simply empty, throwing an exception.
>
> Interestingly, a newline seems to be required.
>
> Using http://bad.http.response.tobias.gr as a substitute server triggers
> it. http://no.http.response.tobias.gr does not.
>
>> Usually that is a sign of a broken HTTP server.
>
> I think it's actually something in-between me and the server. I'll take
> a closer look next time this happens.
>
>> Of course it’s  widespread enough, we’d better handle it, either in 
>> Guix or directly in (web client) in Guile;
>
> As I read it, (web client) considers throwing a BAD-RESPONSE exception
> the best or only way to deal with an error like this. I agree.
>
>> OTOH, if it’s a genuine problem, we’d better not hide it.
>
> Well, we don't hide it, per se. Hence the error message.
>
> I think throwing an unhandled exception is definitely the wrong thing to
> do here — this kills even ‘guix --keep-going --fallback’. I'm less sure
> about the right place to do it

Oh right, if that kills --fallback, that’s a problem.

Back to your initial patch, what about moving ‘bad-response’ handling to
the call site of ‘http-multiple-get’ instead of having it in
‘http-multiple-get’?  (That way, ‘http-multiple-get’ would behave like
‘http-get’ in this respect.)

Upon a ‘bad-response’, ‘fetch-narinfos’ would return #f or the empty
list or the partial narinfo list it has built so far.

WDYT?

I’d be happy with a patch along these lines!

Thank you,
Ludo’.

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 26489 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: bug#26489: [PATCH] substitute: Ignore bad responses.
Date: Fri, 18 Dec 2020 21:19:16 +0100

Hi Tobias,

Your patch #26489 sent a couple of years ago since lost in the vacuum of
the crazy Debbugs.  Patch is here:

   <http://issues.guix.gnu.org/issue/26489>

On Mon, 01 May 2017 at 15:14, ludo <at> gnu.org (Ludovic Courtès) wrote:

>> I think throwing an unhandled exception is definitely the wrong thing to
>> do here — this kills even ‘guix --keep-going --fallback’. I'm less sure
>> about the right place to do it
>
> Oh right, if that kills --fallback, that’s a problem.
>
> Back to your initial patch, what about moving ‘bad-response’ handling to
> the call site of ‘http-multiple-get’ instead of having it in
> ‘http-multiple-get’?  (That way, ‘http-multiple-get’ would behave like
> ‘http-get’ in this respect.)
>
> Upon a ‘bad-response’, ‘fetch-narinfos’ would return #f or the empty
> list or the partial narinfo list it has built so far.
>
> WDYT?
>
> I’d be happy with a patch along these lines!

Tobias, do you plan to rework/rebase it?


All the best,
simon

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 26489 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>
Subject: Re: bug#26489: [PATCH] substitute: Ignore bad responses.
Date: Sun, 20 Dec 2020 14:39:48 +0100

Hi,

zimoun <zimon.toutoune <at> gmail.com> skribis:

> On Mon, 01 May 2017 at 15:14, ludo <at> gnu.org (Ludovic Courtès) wrote:
>
>>> I think throwing an unhandled exception is definitely the wrong thing to
>>> do here — this kills even ‘guix --keep-going --fallback’. I'm less sure
>>> about the right place to do it
>>
>> Oh right, if that kills --fallback, that’s a problem.
>>
>> Back to your initial patch, what about moving ‘bad-response’ handling to
>> the call site of ‘http-multiple-get’ instead of having it in
>> ‘http-multiple-get’?  (That way, ‘http-multiple-get’ would behave like
>> ‘http-get’ in this respect.)
>>
>> Upon a ‘bad-response’, ‘fetch-narinfos’ would return #f or the empty
>> list or the partial narinfo list it has built so far.
>>
>> WDYT?
>>
>> I’d be happy with a patch along these lines!
>
> Tobias, do you plan to rework/rebase it?

In the meantime, commit 5ff521452b9ec2aae9ed8e4bb7bdc250a581f203 added
‘bad-response’ handling (and more).  We should take a closer look, but
it may be that this issue is now addressed.

Ludo’.

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 26489 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>
Subject: Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
Date: Mon, 11 Jan 2021 14:08:22 +0100

Hi Tobias,

On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo <at> gnu.org> wrote:

>>> I’d be happy with a patch along these lines!
>>
>> Tobias, do you plan to rework/rebase it?
>
> In the meantime, commit 5ff521452b9ec2aae9ed8e4bb7bdc250a581f203 added
> ‘bad-response’ handling (and more).  We should take a closer look, but
> it may be that this issue is now addressed.

Since you reported the use-case and the patch, could you confirm that
the commit 5ff521 addresses the issue?

I think it does but maybe I am missing something; probably I am. :-)


Cheers,
simon

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

From: Julien Lepiller <julien <at> lepiller.eu>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 26489 <at> debbugs.gnu.org
Subject: Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
Date: Fri, 15 Jan 2021 20:52:24 +0100

Le Mon, 11 Jan 2021 14:08:22 +0100,
zimoun <zimon.toutoune <at> gmail.com> a écrit :

> Hi Tobias,
> 
> On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo <at> gnu.org> wrote:
> 
> >>> I’d be happy with a patch along these lines!  
> >>
> >> Tobias, do you plan to rework/rebase it?  
> >
> > In the meantime, commit 5ff521452b9ec2aae9ed8e4bb7bdc250a581f203
> > added ‘bad-response’ handling (and more).  We should take a closer
> > look, but it may be that this issue is now addressed.  
> 
> Since you reported the use-case and the patch, could you confirm that
> the commit 5ff521 addresses the issue?
> 
> I think it does but maybe I am missing something; probably I am. :-)
> 
> 
> Cheers,
> simon
> 
> 
> 

It seems not, since a few days ago we had exactly that problem. See
https://issues.guix.gnu.org/45828

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Julien Lepiller <julien <at> lepiller.eu>, Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 26489 <at> debbugs.gnu.org
Subject: Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
Date: Tue, 09 Feb 2021 02:05:34 +0100

Hi,

On Fri, 15 Jan 2021 at 20:52, Julien Lepiller <julien <at> lepiller.eu> wrote:
> Le Mon, 11 Jan 2021 14:08:22 +0100,
> zimoun <zimon.toutoune <at> gmail.com> a écrit :
>> On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> 
>> >>> I’d be happy with a patch along these lines!  
>> >>
>> >> Tobias, do you plan to rework/rebase it?  
>> >
>> > In the meantime, commit 5ff521452b9ec2aae9ed8e4bb7bdc250a581f203
>> > added ‘bad-response’ handling (and more).  We should take a closer
>> > look, but it may be that this issue is now addressed.  
>> 
>> Since you reported the use-case and the patch, could you confirm that
>> the commit 5ff521 addresses the issue?
>> 
>> I think it does but maybe I am missing something; probably I am. :-)
>
> It seems not, since a few days ago we had exactly that problem. See
> https://issues.guix.gnu.org/45828

Bug#45828 is now closed.  I think it makes sense to close this one too.

If no objection, I am proposing to close this old bug. :-)


Cheers,
simon

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Julien Lepiller <julien <at> lepiller.eu>, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 26489-done <at> debbugs.gnu.org
Subject: Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
Date: Tue, 09 Feb 2021 09:42:27 +0100

Hi,

zimoun <zimon.toutoune <at> gmail.com> skribis:

> On Fri, 15 Jan 2021 at 20:52, Julien Lepiller <julien <at> lepiller.eu> wrote:
>> Le Mon, 11 Jan 2021 14:08:22 +0100,
>> zimoun <zimon.toutoune <at> gmail.com> a écrit :
>>> On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo <at> gnu.org> wrote:
>>> 
>>> >>> I’d be happy with a patch along these lines!  
>>> >>
>>> >> Tobias, do you plan to rework/rebase it?  
>>> >
>>> > In the meantime, commit 5ff521452b9ec2aae9ed8e4bb7bdc250a581f203
>>> > added ‘bad-response’ handling (and more).  We should take a closer
>>> > look, but it may be that this issue is now addressed.  
>>> 
>>> Since you reported the use-case and the patch, could you confirm that
>>> the commit 5ff521 addresses the issue?
>>> 
>>> I think it does but maybe I am missing something; probably I am. :-)
>>
>> It seems not, since a few days ago we had exactly that problem. See
>> https://issues.guix.gnu.org/45828
>
> Bug#45828 is now closed.  I think it makes sense to close this one too.
>
> If no objection, I am proposing to close this old bug. :-)

Sounds good, done!

Ludo’.

Previous Next

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