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

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 09 Feb 2021 08:43:01 +0000

Your bug report

#26489: [PATCH] substitute: Ignore bad responses.

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
26489: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=26489
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [bug#26489] [PATCH] substitute: Ignore bad responses.
From: Ludovic Courtès <[email-masked]>
To: zimoun <[email-masked]>
Cc: Julien Lepiller <[email-masked]>, Tobias Geerinckx-Rice <[email-masked]>, [email-masked]
Date: Tue, 09 Feb 2021 09:42:27 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

zimoun <[email-masked]> skribis:

> On Fri, 15 Jan 2021 at 20:52, Julien Lepiller <[email-masked]> wrote:
>> Le Mon, 11 Jan 2021 14:08:22 +0100,
>> zimoun <[email-masked]> a écrit :
>>> On Sun, 20 Dec 2020 at 14:39, Ludovic Courtès <[email-masked]> 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’.

Subject: [PATCH] substitute: Ignore bad responses.
From: Tobias Geerinckx-Rice <[email-masked]>
To: [email-masked]
Date: Fri, 14 Apr 2017 02:27:55 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

* 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