Ludovic Courtès <ludovic.courtes@inria.fr> writes:

> As reported by a few people on IRC, ‘guix substitute’ sometimes fails in
> a way that I just experienced (from
> 8154beffd8c121e953a7c4cd75c3eebfcc073a9a):
>
> --8<---------------cut here---------------start------------->8---
> downloading from https://ci.guix.gnu.org/nar/gzip/0bji0q5n59595xaqkqrp2gv52lbz55xz-libpng-1.6.37 .
>  libpng-1.6.37  275KiB                                 11.0MiB/s 00:00 [##################] 100.0%
>
> downloading from https://ci.guix.gnu.org/nar/lzip/h3a5ygxxh4gakhnl53mq7z9b43l8z05g-python-minimal.
>  python-minimal-wrapper-3.8.2  351B                     293KiB/s 00:00 [##################] 100.0%
>
> downloading from https://ci.guix.gnu.org/nar/lzip/h8j09yb5d8dh3jffvpzawxslig9bwhdr-freetype-2.10..
>  freetype-2.10.4  600KiB                                3.0MiB/s 00:00 [##################] 100.0%
>
> building /gnu/store/2wfzazqz9g5xizi4vq4pv75nkh1m24bp-perl-5.30.2.drv...
> Backtrace:
> In guix/ui.scm:
>   2164:12 19 (run-guix-command _ . _)
> In guix/scripts/substitute.scm:
>     691:2 18 (guix-substitute . _)
> In unknown file:
>           17 (with-continuation-barrier #<procedure thunk ()>)
> In ice-9/boot-9.scm:
>   1736:10 16 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
> In unknown file:
>           15 (apply-smob/0 #<thunk 7fcb3b3594a0>)
> In ice-9/boot-9.scm:
>   1736:10 14 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
>   1736:10 13 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
>   1731:15 12 (with-exception-handler #<procedure 7fcb3a7c4150 at ice-9/boot-9.scm:1815:7 (exn)> _)
> In guix/scripts/substitute.scm:
>    740:17 11 (_)
>     434:7 10 (process-substitution _ "/gnu/store/ns00dyapjbq9037dwrxa7hc31dvir00n-grub-minimal-2.)
> In ice-9/boot-9.scm:
>   1736:10  9 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
> In guix/scripts/substitute.scm:
>     443:9  8 (_)
> In ice-9/boot-9.scm:
>   1731:15  7 (with-exception-handler #<procedure 7fcb3a7d37b0 at ice-9/boot-9.scm:1815:7 (exn)> _)
>   1669:16  6 (raise-exception _ #:continuable? _)
>   1667:16  5 (raise-exception _ #:continuable? _)
>   1669:16  4 (raise-exception _ #:continuable? _)
>   1764:13  3 (_ #<&compound-exception components: (#<&error> #<&irritants irritants: (read-header)
>   1669:16  2 (raise-exception _ #:continuable? _)
>   1667:16  1 (raise-exception _ #:continuable? _)
>   1669:16  0 (raise-exception _ #:continuable? _)
>
> ice-9/boot-9.scm:1669:16: In procedure raise-exception:
> Bad Read-Header-Line header: #<eof>
> --8<---------------cut here---------------end--------------->8---
>
> This is the kind of issue that ‘with-cached-connection’ as it can be
> seen in 9158020d7853b6e7925802e0d0a082801c680e8f avoided:
>
> --8<---------------cut here---------------start------------->8---
> (define* (call-with-cached-connection uri proc
>                                       #:optional
>                                       (open-connection
>                                        open-connection-for-uri/cached))
>   (let ((port (open-connection uri)))
>     (catch #t
>       (lambda ()
>         (proc port))
>       (lambda (key . args)
>         ;; If PORT was cached and the server closed the connection in the
>         ;; meantime, we get EPIPE.  In that case, open a fresh connection and
>         ;; retry.  We might also get 'bad-response or a similar exception from
>         ;; (web response) later on, once we've sent the request, or a
>         ;; ERROR/INVALID-SESSION from GnuTLS.
>         (if (or (and (eq? key 'system-error)
>                      (= EPIPE (system-error-errno `(,key ,@args))))
>                 (and (eq? key 'gnutls-error)
>                      (eq? (first args) error/invalid-session))
>                 (memq key '(bad-response bad-header bad-header-component)))
>             (proc (open-connection uri #:fresh? #t))
>             (apply throw key args))))))
> --8<---------------cut here---------------end--------------->8---
>
> I think 7b812f7c84c43455cdd68a0e51b6ded018afcc8e and subsequent commits
> may have caused this regression.  In particular, in
> 20c08a8a45d0f137ead7c05e720456b2aea44402,
> ‘call-with-connection-error-handling’ is now used, but that one doesn’t
> catch the exceptions mentioned above, in this case ‘bad-header’.

I think the behaviour changed unintentionally with [1], however,
thinking about the connection reuse in process-substitution compared
with http-multiple-get, there's no attempt here to look at if the server
has specified whether the connection should be closed.

1: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=f50f5751fff4cfc6d5abba9681054569694b7a5c

Just like http-multiple-get, it's probably worth trying to check the
headers of the response, look at whether the server has indicated that
the connection should be closed, and if so, close the connection,
forcing a new one to be established for future requests.

I haven't tested this theory, but maybe if that happened, then some
occurrences of trying to read a response, and not being able to would be
prevented.