* bug#26489: [PATCH] substitute: Ignore bad responses.
@ 2017-04-14 0:27 Tobias Geerinckx-Rice
2017-04-14 9:54 ` Ludovic Courtès
0 siblings, 1 reply; 10+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-04-14 0:27 UTC (permalink / raw)
To: 26489
* 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#26489: [PATCH] substitute: Ignore bad responses.
2017-04-14 0:27 bug#26489: [PATCH] substitute: Ignore bad responses Tobias Geerinckx-Rice
@ 2017-04-14 9:54 ` Ludovic Courtès
2017-04-28 20:56 ` Tobias Geerinckx-Rice
0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2017-04-14 9:54 UTC (permalink / raw)
To: Tobias Geerinckx-Rice; +Cc: 26489
Howdy!
Tobias Geerinckx-Rice <me@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’.
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#26489: [PATCH] substitute: Ignore bad responses.
2017-04-14 9:54 ` Ludovic Courtès
@ 2017-04-28 20:56 ` Tobias Geerinckx-Rice
2017-05-01 13:14 ` Ludovic Courtès
0 siblings, 1 reply; 10+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-04-28 20:56 UTC (permalink / raw)
To: ludo; +Cc: 26489
[-- Attachment #1.1: Type: text/plain, Size: 1959 bytes --]
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@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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#26489: [PATCH] substitute: Ignore bad responses.
2017-04-28 20:56 ` Tobias Geerinckx-Rice
@ 2017-05-01 13:14 ` Ludovic Courtès
2020-12-18 20:19 ` [bug#26489] " zimoun
0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2017-05-01 13:14 UTC (permalink / raw)
To: Tobias Geerinckx-Rice; +Cc: 26489
Hi!
Tobias Geerinckx-Rice <me@tobias.gr> skribis:
> On 14/04/17 11:54, Ludovic Courtès wrote:
>> Tobias Geerinckx-Rice <me@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’.
^ permalink raw reply [flat|nested] 10+ messages in thread* [bug#26489] [PATCH] substitute: Ignore bad responses.
2017-05-01 13:14 ` Ludovic Courtès
@ 2020-12-18 20:19 ` zimoun
2020-12-20 13:39 ` Ludovic Courtès
0 siblings, 1 reply; 10+ messages in thread
From: zimoun @ 2020-12-18 20:19 UTC (permalink / raw)
To: Tobias Geerinckx-Rice; +Cc: 26489
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@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#26489] [PATCH] substitute: Ignore bad responses.
2020-12-18 20:19 ` [bug#26489] " zimoun
@ 2020-12-20 13:39 ` Ludovic Courtès
2021-01-11 13:08 ` zimoun
0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2020-12-20 13:39 UTC (permalink / raw)
To: zimoun; +Cc: 26489
Hi,
zimoun <zimon.toutoune@gmail.com> skribis:
> On Mon, 01 May 2017 at 15:14, ludo@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’.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#26489] [PATCH] substitute: Ignore bad responses.
2020-12-20 13:39 ` Ludovic Courtès
@ 2021-01-11 13:08 ` zimoun
2021-01-15 19:52 ` Julien Lepiller
0 siblings, 1 reply; 10+ messages in thread
From: zimoun @ 2021-01-11 13:08 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 26489
Hi Tobias,
On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#26489] [PATCH] substitute: Ignore bad responses.
2021-01-11 13:08 ` zimoun
@ 2021-01-15 19:52 ` Julien Lepiller
2021-02-09 1:05 ` zimoun
0 siblings, 1 reply; 10+ messages in thread
From: Julien Lepiller @ 2021-01-15 19:52 UTC (permalink / raw)
To: zimoun; +Cc: 26489
Le Mon, 11 Jan 2021 14:08:22 +0100,
zimoun <zimon.toutoune@gmail.com> a écrit :
> Hi Tobias,
>
> On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#26489] [PATCH] substitute: Ignore bad responses.
2021-01-15 19:52 ` Julien Lepiller
@ 2021-02-09 1:05 ` zimoun
2021-02-09 8:42 ` bug#26489: " Ludovic Courtès
0 siblings, 1 reply; 10+ messages in thread
From: zimoun @ 2021-02-09 1:05 UTC (permalink / raw)
To: Julien Lepiller, Tobias Geerinckx-Rice; +Cc: Ludovic Courtès, 26489
Hi,
On Fri, 15 Jan 2021 at 20:52, Julien Lepiller <julien@lepiller.eu> wrote:
> Le Mon, 11 Jan 2021 14:08:22 +0100,
> zimoun <zimon.toutoune@gmail.com> a écrit :
>> On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#26489: [PATCH] substitute: Ignore bad responses.
2021-02-09 1:05 ` zimoun
@ 2021-02-09 8:42 ` Ludovic Courtès
0 siblings, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2021-02-09 8:42 UTC (permalink / raw)
To: zimoun; +Cc: Julien Lepiller, Tobias Geerinckx-Rice, 26489-done
Hi,
zimoun <zimon.toutoune@gmail.com> skribis:
> On Fri, 15 Jan 2021 at 20:52, Julien Lepiller <julien@lepiller.eu> wrote:
>> Le Mon, 11 Jan 2021 14:08:22 +0100,
>> zimoun <zimon.toutoune@gmail.com> a écrit :
>>> On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo@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’.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-09 9:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-14 0:27 bug#26489: [PATCH] substitute: Ignore bad responses Tobias Geerinckx-Rice
2017-04-14 9:54 ` Ludovic Courtès
2017-04-28 20:56 ` Tobias Geerinckx-Rice
2017-05-01 13:14 ` Ludovic Courtès
2020-12-18 20:19 ` [bug#26489] " zimoun
2020-12-20 13:39 ` Ludovic Courtès
2021-01-11 13:08 ` zimoun
2021-01-15 19:52 ` Julien Lepiller
2021-02-09 1:05 ` zimoun
2021-02-09 8:42 ` bug#26489: " Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).