[PATCH] substitute: Ignore bad responses.

DoneSubmitted by Tobias Geerinckx-Rice.
Details
4 participants
  • Julien Lepiller
  • Ludovic Courtès
  • Tobias Geerinckx-Rice
  • zimoun
Owner
unassigned
Severity
normal
T
T
Tobias Geerinckx-Rice wrote on 14 Apr 2017 02:27
Download
(address . guix-patches@gnu.org)
20170414002755.32672-1-me@tobias.gr
Download
* guix/scripts/substitute.scm (http-multiple-get): Catch BAD-RESPONSEexceptions 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 .narinfowill 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 obviousthat only READ-RESPONSE can throw, and that PROC will never be calledwith, a bad response? No idea. I haven't had enough free time to learngood 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(-)
Toggle diff (43 lines)diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scmindex 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
L
L
Ludovic Courtès wrote on 14 Apr 2017 11:54
Download
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 26489@debbugs.gnu.org)
87lgr3gvqf.fsf@gnu.org
Download
Howdy!
Tobias Geerinckx-Rice <me@tobias.gr> skribis:
Toggle quote (18 lines)> * 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 inpractice? :-)
Usually that is a sign of a broken HTTP server. Of course it’swidespread 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 nothide it.
Thanks,Ludo’.
T
T
Tobias Geerinckx-Rice wrote on 28 Apr 2017 22:56
Download
(address . ludo@gnu.org)(address . 26489@debbugs.gnu.org)
ca4baf38-3b11-1c8a-a7b1-167f6500c247@tobias.gr
Download
Ludo',
I should really send this message in my Drafts folder since 14/4... :-/
On 14/04/17 11:54, Ludovic Courtès wrote:
Toggle quote (16 lines)> 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: theHTTP response (the first line returned to the client, e.g. "HTTP/1.1 200OK") was simply empty, throwing an exception.
Interestingly, a newline seems to be required.
Using http://bad.http.response.tobias.gras a substitute server triggersit. http://no.http.response.tobias.grdoes not.
Toggle quote (2 lines)> Usually that is a sign of a broken HTTP server.
I think it's actually something in-between me and the server. I'll takea closer look next time this happens.
Toggle quote (3 lines)> 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 exceptionthe best or only way to deal with an error like this. I agree.
Toggle quote (2 lines)> 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 todo here — this kills even ‘guix --keep-going --fallback’. I'm less sureabout the right place to do it
Kind regards,
T G-R
Attachment: signature.asc
L
L
Ludovic Courtès wrote on 1 May 2017 15:14
Download
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 26489@debbugs.gnu.org)
87o9vc3eij.fsf@gnu.org
Download
Hi!
Tobias Geerinckx-Rice <me@tobias.gr> skribis:
Toggle quote (45 lines)> 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 tothe 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 emptylist or the partial narinfo list it has built so far.
WDYT?
I’d be happy with a patch along these lines!
Thank you,Ludo’.
Z
Z
zimoun wrote on 18 Dec 2020 21:19
Download
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
861rfmvpdn.fsf@gmail.com
Download
Hi Tobias,
Your patch #26489 sent a couple of years ago since lost in the vacuum ofthe 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:
Toggle quote (18 lines)>> 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
L
L
Ludovic Courtès wrote on 20 Dec 2020 14:39
Download
(name . zimoun)(address . zimon.toutoune@gmail.com)
87o8io8ul7.fsf@gnu.org
Download
Hi,
zimoun <zimon.toutoune@gmail.com> skribis:
Toggle quote (22 lines)> 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, butit may be that this issue is now addressed.
Ludo’.
Z
Z
zimoun wrote on 11 Jan 14:08 +0100
Download
Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
(name . Ludovic Courtès)(address . ludo@gnu.org)
861rertyd5.fsf@gmail.com
Download
Hi Tobias,
On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (8 lines)>>> 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 thatthe commit 5ff521 addresses the issue?
I think it does but maybe I am missing something; probably I am. :-)

Cheers,simon
J
J
Julien Lepiller wrote on 15 Jan 20:52 +0100
Download
(name . zimoun)(address . zimon.toutoune@gmail.com)
20210115205224.7f775008@tachikoma.lepiller.eu
Download
Le Mon, 11 Jan 2021 14:08:22 +0100,zimoun <zimon.toutoune@gmail.com> a écrit :
Toggle quote (24 lines)> 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. Seehttps://issues.guix.gnu.org/45828
Z
Z
zimoun wrote on 9 Feb 02:05 +0100
Download
86v9b2jbj5.fsf@gmail.com
Download
Hi,
On Fri, 15 Jan 2021 at 20:52, Julien Lepiller <julien@lepiller.eu> wrote:
Toggle quote (20 lines)> 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
L
L
Ludovic Courtès wrote on 9 Feb 09:42 +0100
Download
(name . zimoun)(address . zimon.toutoune@gmail.com)
87h7mlvdho.fsf@gnu.org
Download
Hi,
zimoun <zimon.toutoune@gmail.com> skribis:
Toggle quote (25 lines)> 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’.
Closed
?
Your may also send email to 26489@debbugs.gnu.org to comment.