GNU bug report logs - #47288
[PATCH] guix: http-client: Tweak http-multiple-get error handling.

Previous Next

Full log

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 47288 <at> debbugs.gnu.org
Subject: Re: bug#47288: [PATCH] guix: http-client: Tweak http-multiple-get
 error handling.
Date: Sat, 27 Mar 2021 18:15:42 +0100

Hi,

Christopher Baines <mail <at> cbaines.net> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:

[...]

>> +           (match (false-if-networking-error (read-response p))
>> +             ((? response? resp)
>> +              (let* ((body   (response-body-port resp))
>> +                     (result (proc head resp body result)))
>
> Given body is a port, and that port is passed to proc, I'm guessing it's
> possible for networking things to go wrong inside proc.

Yes, but how is this different from a regular read(2) call as made by
‘get-bytevector-n’ or whatever?  We wouldn’t write every read(2) call in
‘catch’ because in general any error there is indeed exceptional.

I think the only bit that’s “less exceptional” here is that, because
we’re reusing cached connection, we know that the first read(2) or the
first write(2) to that port can trigger one of these errors—which, we
know are not “exceptional”.  Errors in subsequent read(2) or write(2)
calls remain exceptional/unrecoverable and should be treated as such
IMO.

Does that make sense?

(In that sense, I think wrapping every ‘read-response’ call rather than
just the first one is already too much, but that’s okay.)

>> +                ;; 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-port p)
>> +                   (connect #f                    ;try again
>> +                            (drop requests (+ 1 processed))
>> +                            result))
>> +                  (_
>> +                   (loop tail (+ 1 processed) result)))))
>> +             (#f
>> +              (close-port p)
>> +              (connect #f                         ; try again
>> +                       (drop requests (+ 1 processed))
>
> I realised earlier in this series of patches that this should actually
> be processed, rather than (+ 1 processed) since proc can't have been run
> for the current response.

Oh, something to fix in a subsequent commit, then.

All in all, I propose to go with this patch if that’s fine with you.

Thanks!

Ludo’.

Previous Next

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