--- Begin Message ---
|
Subject: |
[PATCH] scripts: substitute: Add back some error handling. |
|
Date: |
Mon, 15 Mar 2021 15:11:33 +0000 |
In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called within
process-substitution was changed. As with-cached-connection actually includes
important error handling for the opening of a HTTP request (when using a
cached connection), this change removed some error handling.
This commit adds that error handling back,
with-cached-connection/call-with-cached-connection is back, rebranded as
call-with-fresh-connection-retry.
* guix/scripts/substitute.scm (process-substitution): Retry once for some
errors when making HTTP requests to fetch substitutes.
---
guix/scripts/substitute.scm | 38 ++++++++++++++++++++++++++++++++-----
1 file changed, 33 insertions(+), 5 deletions(-)
diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index 6892aa999b..2c9b45023f 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -45,6 +45,7 @@
#:select (uri-abbreviation nar-uri-abbreviation
(open-connection-for-uri
. guix:open-connection-for-uri)))
+ #:autoload (gnutls) (error/invalid-session)
#:use-module (guix progress)
#:use-module ((guix build syscalls)
#:select (set-thread-name))
@@ -401,6 +402,31 @@ the current output port."
(apply dump-file/deduplicate
(append args (list #:store (%store-prefix)))))
+ (define (call-with-fresh-connection-retry uri proc)
+ (define (get-port)
+ (open-connection-for-uri/cached uri
+ #:verify-certificate? #f))
+
+ (let ((port (get-port)))
+ (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)))
+ (begin
+ (close-port port) ; close the port to get a fresh one
+ (proc (get-port)))
+ (apply throw key args))))))
+
(define (fetch uri)
(case (uri-scheme uri)
((file)
@@ -424,11 +450,13 @@ the current output port."
(call-with-connection-error-handling
uri
(lambda ()
- (http-fetch uri #:text? #f
- #:open-connection open-connection-for-uri/cached
- #:keep-alive? #t
- #:buffered? #f
- #:verify-certificate? #f))))))
+ (call-with-fresh-connection-retry
+ uri
+ (lambda (port)
+ (http-fetch uri #:text? #f
+ #:port port
+ #:keep-alive? #t
+ #:buffered? #f))))))))
(else
(leave (G_ "unsupported substitute URI scheme: ~a~%")
(uri->string uri)))))
--
2.30.1
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#47160: [PATCH] scripts: substitute: Add back some error handling. |
|
Date: |
Wed, 17 Mar 2021 20:46:05 +0000 |
|
User-agent: |
mu4e 1.4.15; emacs 27.1 |
Ludovic Courtès <ludo@gnu.org> writes:
> Howdy!
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called within
>> process-substitution was changed. As call-with-cached-connection actually
>> includes important error handling for the opening of a HTTP request, this
>> change removed some error handling. This commit adds that back.
>>
>> Fixes <https://bugs.gnu.org/47157>.
>>
>> * guix/scripts/substitute.scm (call-with-cached-connection): New procedure.
>> (with-cached-connection): New syntax rule.
>> (process-substitution): Retry once for some errors when making HTTP requests
>> to fetch substitutes.
>
> [...]
>
>> The call-with-connection-error-handling was added in
>> 20c08a8a45d0f137ead7c05e720456b2aea44402, but that error handling was
>> previously inside of open-connection-for-uri/maybe, which is related
>> to (call-)with-cached-connection which was used in process-substitution, but
>> only actually used with call-with-cached-connection when used in
>> fetch-narinfos.
>>
>> There's some handling for similar errors within with-networking, which is
>> used
>> within process-substitution.
>>
>> * guix/scripts/substitute.scm (process-substitution): Remove
>> call-with-connection-error-handling call.
>
> Both LGTM, thank you!
Great, pushed as b48204259aa9cad80c5b23a4060e2d796007ec7a.
Note that this won't have any affect on the substitute script for most
users until the guix package is updated to include these changes.
Chris
signature.asc
Description: PGP signature
--- End Message ---