bug#26489: closed ([PATCH] substitute: Ignore bad responses.)

[GNU bug Tracking System]

Your message dated Tue, 09 Feb 2021 09:42:27 +0100
with message-id <87h7mlvdho.fsf@gnu.org>
and subject line Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
has caused the debbugs.gnu.org bug report #26489,
regarding [PATCH] substitute: Ignore bad responses.
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
26489: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=26489
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems
> --- Begin Message ---
>
>
>
> 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
>
>
>
>
> --- End Message ---
> --- Begin Message ---
>
>
>
> Subject: 
>
> Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
>
>
>
>
> Date: 
>
> Tue, 09 Feb 2021 09:42:27 +0100
>
>
>
>
> User-agent: 
>
> Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
>
>
>
>
> 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’.
>
>
> --- End Message ---