| Message ID | 20210316234628.24479-1-mail@cbaines.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#47160,1/2] scripts: substitute: Add back some error handling. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
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! Ludo’.
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
diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm index 6892aa999b..812f2999ab 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)) @@ -377,6 +378,32 @@ server certificates." (drain-input socket) socket)))))))) +(define (call-with-cached-connection uri proc) + (let ((port (open-connection-for-uri/cached uri + #:verify-certificate? #f))) + (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-for-uri/cached uri + #:verify-certificate? #f + #:fresh? #t)) + (apply throw key args)))))) + +(define-syntax-rule (with-cached-connection uri port exp ...) + "Bind PORT with EXP... to a socket connected to URI." + (call-with-cached-connection uri (lambda (port) exp ...))) + (define* (process-substitution store-item destination #:key cache-urls acl deduplicate? print-build-trace?) @@ -424,11 +451,11 @@ 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)))))) + (with-cached-connection uri port + (http-fetch uri #:text? #f + #:port port + #:keep-alive? #t + #:buffered? #f))))))) (else (leave (G_ "unsupported substitute URI scheme: ~a~%") (uri->string uri))))) @@ -715,6 +742,8 @@ if needed, as expected by the daemon's agent." ;;; Local Variables: ;;; eval: (put 'with-timeout 'scheme-indent-function 1) ;;; eval: (put 'with-redirected-error-port 'scheme-indent-function 0) +;;; eval: (put 'with-cached-connection 'scheme-indent-function 2) +;;; eval: (put 'call-with-cached-connection 'scheme-indent-function 1) ;;; End: ;;; substitute.scm ends here