From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 15:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47160@debbugs.gnu.org X-Debbugs-Original-To: guix-patches@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.161582109922411 (code B ref -1); Mon, 15 Mar 2021 15:12:02 +0000 Received: (at submit) by debbugs.gnu.org; 15 Mar 2021 15:11:39 +0000 Received: from localhost ([127.0.0.1]:36361 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLosd-0005pP-BS for submit@debbugs.gnu.org; Mon, 15 Mar 2021 11:11:39 -0400 Received: from lists.gnu.org ([209.51.188.17]:37518) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLosb-0005pG-Ex for submit@debbugs.gnu.org; Mon, 15 Mar 2021 11:11:37 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38552) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lLosb-000832-9h for guix-patches@gnu.org; Mon, 15 Mar 2021 11:11:37 -0400 Received: from mira.cbaines.net ([2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27]:38603) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lLosZ-0005u2-Cn for guix-patches@gnu.org; Mon, 15 Mar 2021 11:11:37 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 362DA27BC52 for ; Mon, 15 Mar 2021 15:11:33 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 8c7d25df for ; Mon, 15 Mar 2021 15:11:33 +0000 (UTC) From: Christopher Baines Date: Mon, 15 Mar 2021 15:11:33 +0000 Message-Id: <20210315151133.16282-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.4 (-) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.4 (--) 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 From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 15:21:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161582165623426 (code B ref 47160); Mon, 15 Mar 2021 15:21:02 +0000 Received: (at 47160) by debbugs.gnu.org; 15 Mar 2021 15:20:56 +0000 Received: from localhost ([127.0.0.1]:36388 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLp1c-00065m-0X for submit@debbugs.gnu.org; Mon, 15 Mar 2021 11:20:56 -0400 Received: from eggs.gnu.org ([209.51.188.92]:51202) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLp1a-00065X-Co for 47160@debbugs.gnu.org; Mon, 15 Mar 2021 11:20:54 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:43885) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lLp1T-0000Nd-Px; Mon, 15 Mar 2021 11:20:47 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=46140 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lLp1R-0003kC-Vv; Mon, 15 Mar 2021 11:20:47 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210315151133.16282-1-mail@cbaines.net> Date: Mon, 15 Mar 2021 16:20:43 +0100 In-Reply-To: <20210315151133.16282-1-mail@cbaines.net> (Christopher Baines's message of "Mon, 15 Mar 2021 15:11:33 +0000") Message-ID: <8735wwh29g.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Hi, Christopher Baines skribis: > In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called wit= hin > process-substitution was changed. As with-cached-connection actually inc= ludes > 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. [...] > + (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 connect= ion > + ;; 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) > + (=3D 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-compone= nt))) > + (begin > + (close-port port) ; close the port to get a fresh = one > + (proc (get-port))) > + (apply throw key args)))))) I think this should be at the top level for clarity. It used to have =E2=80=98cached=E2=80=99 in its name because catching all these exceptions = is something you wouldn=E2=80=99t normally do; it only makes sense in the context of cac= hed connections. > (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/cach= ed > - #: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)))))))) Does =E2=80=98call-with-connection-error-handling=E2=80=99 still make sense= here? There=E2=80=99s already =E2=80=98with-networking=E2=80=99 at the top level = to do proper networking error reporting. Regarding , I would lean towards perhaps reverting the connection/error-handling patch series and starting anew from that =E2=80=9Cknown state=E2=80=9D. This area is unfortunately quite tedious to test and to get right so I=E2= =80=99d err on the path of conservative, incremental changes. Thought? Ludo=E2=80=99. From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 16:16:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161582490630016 (code B ref 47160); Mon, 15 Mar 2021 16:16:02 +0000 Received: (at 47160) by debbugs.gnu.org; 15 Mar 2021 16:15:06 +0000 Received: from localhost ([127.0.0.1]:36569 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLps2-0007o4-47 for submit@debbugs.gnu.org; Mon, 15 Mar 2021 12:15:06 -0400 Received: from mira.cbaines.net ([212.71.252.8]:33360) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLpry-0007nc-IF for 47160@debbugs.gnu.org; Mon, 15 Mar 2021 12:15:05 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id B5CDF27BC52; Mon, 15 Mar 2021 16:15:01 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 92234124; Mon, 15 Mar 2021 16:15:00 +0000 (UTC) References: <20210315151133.16282-1-mail@cbaines.net> <8735wwh29g.fsf@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines In-reply-to: <8735wwh29g.fsf@gnu.org> Date: Mon, 15 Mar 2021 16:15:00 +0000 Message-ID: <874khcfl6j.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Hi, > > Christopher Baines skribis: > >> In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called wi= thin >> process-substitution was changed. As with-cached-connection actually in= cludes >> 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. > > [...] > >> + (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 connec= tion >> + ;; 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) >> + (=3D 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-compon= ent))) >> + (begin >> + (close-port port) ; close the port to get a fresh= one >> + (proc (get-port))) >> + (apply throw key args)))))) > > I think this should be at the top level for clarity. It used to have > =E2=80=98cached=E2=80=99 in its name because catching all these exception= s is something > you wouldn=E2=80=99t normally do; it only makes sense in the context of c= ached > connections. I initially tried to just put the error handling in just where it's needed, but that was difficult since the http-fetch bit needs to happen again when there's a relevant error. The two things: getting a port which maybe is a cached connection and handling some errors plus potentially re-running proc is difficult to capture in a name, but "call-with-cached-connection-and-error-handling" is an improvement over "with-cached-connection" I think. >> (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/cac= hed >> - #: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)))))))) > > Does =E2=80=98call-with-connection-error-handling=E2=80=99 still make sen= se here? > There=E2=80=99s already =E2=80=98with-networking=E2=80=99 at the top leve= l to do proper > networking error reporting. So, looking back, the call-with-connection-error-handling error handling was related to (call-)with-cached-connection, but it was only relevant inside of fetch-narinfos, as that's when open-connection-for-uri/maybe was passed in to call-with-cached-connection. Which means no, I think it can be removed, at least that's more consistent with the older behaviour. I'll send some updated patches. > Regarding , I would lean towards > perhaps reverting the connection/error-handling patch series and > starting anew from that =E2=80=9Cknown state=E2=80=9D. > > This area is unfortunately quite tedious to test and to get right so I=E2= =80=99d > err on the path of conservative, incremental changes. > > Thought? My preference is still to try and move forward and to make the error handling easier to see in the code. Particularly with this change, I think the problem was introduced in this commit [1], but I think it's hard to tell from the diff, since the error handling and retrying is within with-cached-connection. 1: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=3Df50f5751fff4cfc6= d5abba9681054569694b7a5c That commit was one of the commits where I was making small incremental changes prior to actually getting to the changes I was looking at making, but a breakage was still introduced. What I was thinking about with this patch was how to make the error handling being added back here easier to see, and thus harder to break/remove. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBPiARfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XdzNhAAjlEXfQFwHMnKdBI6XW/16fih/rB9wMcA hkkHtvRyQQyEKkYaGBYhRcO/yYt5ovtjkT+FkcvWgA+MlOo9Fomk7eauyOIZcBA4 ibXRpfli5qvOvqSqr9Diu3lTiMYqIOAy1dG3iqYhbcQL+fNVJo5mI/FyMPSt3o5F EhRZJXeOn3wdV2KOWspRgIHpE43LJWGEiSJ6ISFrrvpHFBBL8Nu8K9nLa3vB+hFt s8tFMh6bityYCrlwHhS5cP0B2nenmI9BwZQzl+doB8I6N3hMfu7OdkUm/kNlqImQ 1MWOA/rYGrQNlj9XOGrefX4zQLyOSyfn3PcTcg4MqY8Az4WaaRmleIPyK5ZW4ikU /OQYBe5A6rz3HOtZxH7yYj8sEEWEWepSn5fwAxv+tAH5Ydpn8Iz91QGkHfty6ApM 6Gr+zn6g9CPvpVG/y16pvQx6nIFCrf5GJQcu8GgdZQn9/1dH/Fpa9KOYGis/X7hV 0rNYWaMq7/hcqA4nINS+JJSB6VSSXeh1+sxPh4GXuKxuev/UKjejfxGMaCZD4v7u LB+WR64S+tFE2frKS38PsXp1XmdmxSgEjTUBu0W+N/JbxpNb32o+XzZOWf1uS1dO VBEg/1WweBpTjQuHdi9TBb7Qhw6SvBK5kNBajwRq4JrdamNzyUm7QI7hzq/WZix9 xx6RtWXJjtw= =xoNb -----END PGP SIGNATURE----- --=-=-=-- From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH v2 2/2] scripts: substitute: Tweak error reporting in process-substitution. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 16:16:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161582493630079 (code B ref 47160); Mon, 15 Mar 2021 16:16:02 +0000 Received: (at 47160) by debbugs.gnu.org; 15 Mar 2021 16:15:36 +0000 Received: from localhost ([127.0.0.1]:36573 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLpsV-0007p3-Mf for submit@debbugs.gnu.org; Mon, 15 Mar 2021 12:15:35 -0400 Received: from mira.cbaines.net ([212.71.252.8]:33368) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLpsT-0007oq-An for 47160@debbugs.gnu.org; Mon, 15 Mar 2021 12:15:33 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id EE96927BC55 for <47160@debbugs.gnu.org>; Mon, 15 Mar 2021 16:15:32 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 694d5986 for <47160@debbugs.gnu.org>; Mon, 15 Mar 2021 16:15:32 +0000 (UTC) From: Christopher Baines Date: Mon, 15 Mar 2021 16:15:32 +0000 Message-Id: <20210315161532.1716-2-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210315161532.1716-1-mail@cbaines.net> References: <20210315161532.1716-1-mail@cbaines.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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. --- guix/scripts/substitute.scm | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm index 16ba28455f..997e2565e0 100755 --- a/guix/scripts/substitute.scm +++ b/guix/scripts/substitute.scm @@ -447,16 +447,13 @@ the current output port." (warning (G_ "while fetching ~a: server is somewhat slow~%") (uri->string uri)) (warning (G_ "try `--no-substitutes' if the problem persists~%"))) - (call-with-connection-error-handling + (call-with-cached-connection-and-error-handling uri - (lambda () - (call-with-cached-connection-and-error-handling - uri - (lambda (port) - (http-fetch uri #:text? #f - #:port port - #:keep-alive? #t - #:buffered? #f)))))))) + (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 From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH v2 1/2] scripts: substitute: Add back some error handling. References: <20210315151133.16282-1-mail@cbaines.net> In-Reply-To: <20210315151133.16282-1-mail@cbaines.net> Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 16:16:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161582493630085 (code B ref 47160); Mon, 15 Mar 2021 16:16:03 +0000 Received: (at 47160) by debbugs.gnu.org; 15 Mar 2021 16:15:36 +0000 Received: from localhost ([127.0.0.1]:36575 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLpsV-0007p6-Vf for submit@debbugs.gnu.org; Mon, 15 Mar 2021 12:15:36 -0400 Received: from mira.cbaines.net ([212.71.252.8]:33366) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLpsT-0007op-9r for 47160@debbugs.gnu.org; Mon, 15 Mar 2021 12:15:33 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id E55A827BC54 for <47160@debbugs.gnu.org>; Mon, 15 Mar 2021 16:15:32 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 1b5c9824 for <47160@debbugs.gnu.org>; Mon, 15 Mar 2021 16:15:32 +0000 (UTC) From: Christopher Baines Date: Mon, 15 Mar 2021 16:15:31 +0000 Message-Id: <20210315161532.1716-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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, (call-)with-cached-connection is back, rebranded as call-with-cached-connection-and-error-handling. * 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..16ba28455f 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,31 @@ server certificates." (drain-input socket) socket)))))))) +(define (call-with-cached-connection-and-error-handling 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* (process-substitution store-item destination #:key cache-urls acl deduplicate? print-build-trace?) @@ -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-cached-connection-and-error-handling + 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 From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 20:52:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.16158414722920 (code B ref 47160); Mon, 15 Mar 2021 20:52:01 +0000 Received: (at 47160) by debbugs.gnu.org; 15 Mar 2021 20:51:12 +0000 Received: from localhost ([127.0.0.1]:37368 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLuBD-0000l2-Vc for submit@debbugs.gnu.org; Mon, 15 Mar 2021 16:51:12 -0400 Received: from eggs.gnu.org ([209.51.188.92]:38332) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLuBC-0000ko-3j for 47160@debbugs.gnu.org; Mon, 15 Mar 2021 16:51:10 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:52173) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lLuB6-0006L1-RA; Mon, 15 Mar 2021 16:51:04 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=46690 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lLuB6-0008PK-1v; Mon, 15 Mar 2021 16:51:04 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210315151133.16282-1-mail@cbaines.net> <8735wwh29g.fsf@gnu.org> <874khcfl6j.fsf@cbaines.net> Date: Mon, 15 Mar 2021 21:51:02 +0100 In-Reply-To: <874khcfl6j.fsf@cbaines.net> (Christopher Baines's message of "Mon, 15 Mar 2021 16:15:00 +0000") Message-ID: <87pn00b0p5.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Christopher Baines skribis: > Ludovic Court=C3=A8s writes: [...] >> Regarding , I would lean towards >> perhaps reverting the connection/error-handling patch series and >> starting anew from that =E2=80=9Cknown state=E2=80=9D. >> >> This area is unfortunately quite tedious to test and to get right so I= =E2=80=99d >> err on the path of conservative, incremental changes. >> >> Thought? > > My preference is still to try and move forward and to make the error > handling easier to see in the code. > > Particularly with this change, I think the problem was introduced in > this commit [1], but I think it's hard to tell from the diff, since the > error handling and retrying is within with-cached-connection. > > 1: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=3Df50f5751fff4cf= c6d5abba9681054569694b7a5c > > That commit was one of the commits where I was making small incremental > changes prior to actually getting to the changes I was looking at > making, but a breakage was still introduced. > > What I was thinking about with this patch was how to make the error > handling being added back here easier to see, and thus harder to > break/remove. OK. Though I=E2=80=99m still unsure what the patch series starting at 7b812f7c84c43455cdd68a0e51b6ded018afcc8e was about. What was the end goal? I also wonder if it introduced other issues. For example, 7b812f7c84c43455cdd68a0e51b6ded018afcc8e replaced a reference to =E2=80=98open-connection-for-uri/cached=E2=80=99 by one to =E2=80=98open-connection-for-uri/maybe=E2=80=99. Are we still using cached= connections? Commit f50f5751fff4cfc6d5abba9681054569694b7a5c no longer passes the #:port parameter to =E2=80=98http-fetch=E2=80=99. Commit 20c08a8a45d0f137ead7c05e720456b2aea44402 does other things but at first sight I=E2=80=99m not sure what the effect is. If you=E2=80=99re confident we can move forward to fix the bug, that=E2=80= =99s great (though we=E2=80=99ll need a good deal of testing), but I=E2=80=99d still l= ike to clarify these points later on. Ludo=E2=80=99. From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 21:34:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.16158440196844 (code B ref 47160); Mon, 15 Mar 2021 21:34:02 +0000 Received: (at 47160) by debbugs.gnu.org; 15 Mar 2021 21:33:39 +0000 Received: from localhost ([127.0.0.1]:37405 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLuqJ-0001mJ-0n for submit@debbugs.gnu.org; Mon, 15 Mar 2021 17:33:39 -0400 Received: from mira.cbaines.net ([212.71.252.8]:37376) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLuqG-0001mA-U0 for 47160@debbugs.gnu.org; Mon, 15 Mar 2021 17:33:37 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id B873427BC52; Mon, 15 Mar 2021 21:33:35 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 82238ee9; Mon, 15 Mar 2021 21:33:34 +0000 (UTC) References: <20210315151133.16282-1-mail@cbaines.net> <8735wwh29g.fsf@gnu.org> <874khcfl6j.fsf@cbaines.net> <87pn00b0p5.fsf_-_@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines In-reply-to: <87pn00b0p5.fsf_-_@gnu.org> Date: Mon, 15 Mar 2021 21:33:31 +0000 Message-ID: <87k0q8drv8.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Christopher Baines skribis: > >> Ludovic Court=C3=A8s writes: > > [...] > >>> Regarding , I would lean towards >>> perhaps reverting the connection/error-handling patch series and >>> starting anew from that =E2=80=9Cknown state=E2=80=9D. >>> >>> This area is unfortunately quite tedious to test and to get right so I= =E2=80=99d >>> err on the path of conservative, incremental changes. >>> >>> Thought? >> >> My preference is still to try and move forward and to make the error >> handling easier to see in the code. >> >> Particularly with this change, I think the problem was introduced in >> this commit [1], but I think it's hard to tell from the diff, since the >> error handling and retrying is within with-cached-connection. >> >> 1: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=3Df50f5751fff4c= fc6d5abba9681054569694b7a5c >> >> That commit was one of the commits where I was making small incremental >> changes prior to actually getting to the changes I was looking at >> making, but a breakage was still introduced. >> >> What I was thinking about with this patch was how to make the error >> handling being added back here easier to see, and thus harder to >> break/remove. > > OK. > > Though I=E2=80=99m still unsure what the patch series starting at > 7b812f7c84c43455cdd68a0e51b6ded018afcc8e was about. What was the end > goal? So that was part of the creation of the (guix substitutes) module, unpicking the code in the script to separate out some of the connection caching was a prerequisite (discussion starts here https://issues.guix.gnu.org/45409#5 ). I think separating out that module is still a good thing. It's allowed for improvements in guix, the weather script doesn't now call in to the substitute script code for example. I'd also like the separation for things like the Guix Build Coordinator, which currently attempts to use the substitute code from Guix. > I also wonder if it introduced other issues. For > example, 7b812f7c84c43455cdd68a0e51b6ded018afcc8e replaced a reference > to =E2=80=98open-connection-for-uri/cached=E2=80=99 by one to > =E2=80=98open-connection-for-uri/maybe=E2=80=99. Are we still using cach= ed > connections? At least on that commit, open-connection-for-uri/maybe calls open-connection-for-uri/cached, so yes, still using cached connections. > Commit f50f5751fff4cfc6d5abba9681054569694b7a5c no longer passes the > #:port parameter to =E2=80=98http-fetch=E2=80=99. Yeah, that change is sort of fine if you're just looking at how the port/connection is handled, but that area is being fixed up here, and because closing the port is something that happens, it's better to also pass the port in. > Commit 20c08a8a45d0f137ead7c05e720456b2aea44402 does other things but at > first sight I=E2=80=99m not sure what the effect is. So, open-connection-for-uri/maybe is like open-connection-for-uri/cached, but it catches a couple of exceptions relating to not being able to connect to a substitute server, it also remembers about showing the messages. The second commit here is changing that slightly, to not apply to process-substitution, however I do think that code might have applied in the past (as open-connection-for-uri/maybe was used I believe). But I think you're right in saying there's probably some overlap between the error handling here and done by with-networking. > If you=E2=80=99re confident we can move forward to fix the bug, that=E2= =80=99s great > (though we=E2=80=99ll need a good deal of testing), but I=E2=80=99d still= like to > clarify these points later on. Well, the changes I'm suggesting here seem reasonable to me. As for testing, checking things basically work is easy enough, but I don't currently have many ideas for how to test for when fetching things doesn't go to plan (which can of course happen). --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBP0qtfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9Xddlg//TR2Bk4rpGZXVPBe9lGJVksjCMUgQi4+k 8gnyLLQj5NxVTlHUe5amThLwqi3AES6VC8r3I18X8ya+zJRSflb6yLawteil3bu7 YQ8J1fOE20EPUK9+A1zSxnBRCA5Mtf6AqmbkxUNyABL08sbEYgJinbjP/TLQuvOt ZQAvEVND/Mdm7UCdSruNQX/G/h2epG6c+6MSAUIb1DTrpPJ7w1sydROTWZzS1lqs Pp6R+B04NndxLoidPfrbFHzM4VAuVgIjynzSUk/EvpdzTqwPLvlG99NMrFQjEmML 4nryWXLEBbNL9w5/lGRhgMTOupMm4Sd4A2c7MO5v41Wzp46y3juIZwGl5AuiJDQp 2YYdDk9ximEymj4RTrE6eFwdd4sJo5NU2DfH82DRQ11Ng7m0VVAG0QvcHzT1UtXi 3btZ+74QIOS1+8+CfvCbIXVC8OoeOh5c3QEBLz/fawPn9iUgi1QHhrxac3XSIZ1T cWPuff+1ykuw0+hSQFi8uBixX9PJiEDRDV650sXff6i/0ZkYw2/DQz+AnoMs4Z/7 xLHco+UFNUMrG0KSipXTmgmwLdGaCtXfOsy5faTWSMZZCD9VjGZqccK+QcnKgGNR s/DfnJL/MVSHFfdWeTArgw38dH2PGOrz/C345hhhACpZND7haw0dm3dW8oxE9OPT 7v99lkZfjhg= =NEX4 -----END PGP SIGNATURE----- --=-=-=-- From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 16 Mar 2021 20:36:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161592690911052 (code B ref 47160); Tue, 16 Mar 2021 20:36:01 +0000 Received: (at 47160) by debbugs.gnu.org; 16 Mar 2021 20:35:09 +0000 Received: from localhost ([127.0.0.1]:41152 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMGPF-0002sB-9r for submit@debbugs.gnu.org; Tue, 16 Mar 2021 16:35:09 -0400 Received: from eggs.gnu.org ([209.51.188.92]:48118) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMGPB-0002rZ-TS for 47160@debbugs.gnu.org; Tue, 16 Mar 2021 16:35:08 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:47335) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lMGP6-0000tj-Dt; Tue, 16 Mar 2021 16:35:00 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=44356 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lMGP4-0002oW-FP; Tue, 16 Mar 2021 16:35:00 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210315151133.16282-1-mail@cbaines.net> <8735wwh29g.fsf@gnu.org> <874khcfl6j.fsf@cbaines.net> <87pn00b0p5.fsf_-_@gnu.org> <87k0q8drv8.fsf@cbaines.net> Date: Tue, 16 Mar 2021 21:34:56 +0100 In-Reply-To: <87k0q8drv8.fsf@cbaines.net> (Christopher Baines's message of "Mon, 15 Mar 2021 21:33:31 +0000") Message-ID: <87zgz27s7j.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Howdy! Christopher Baines skribis: > Ludovic Court=C3=A8s writes: [...] >> Though I=E2=80=99m still unsure what the patch series starting at >> 7b812f7c84c43455cdd68a0e51b6ded018afcc8e was about. What was the end >> goal? > > So that was part of the creation of the (guix substitutes) module, > unpicking the code in the script to separate out some of the connection > caching was a prerequisite (discussion starts here > https://issues.guix.gnu.org/45409#5 ). > > I think separating out that module is still a good thing. It's allowed > for improvements in guix, the weather script doesn't now call in to the > substitute script code for example. I'd also like the separation for > things like the Guix Build Coordinator, which currently attempts to use > the substitute code from Guix. Right, I agree this is a worthy goal. Untangling the stateful bits is the hard part, as we see. :-) >> I also wonder if it introduced other issues. For >> example, 7b812f7c84c43455cdd68a0e51b6ded018afcc8e replaced a reference >> to =E2=80=98open-connection-for-uri/cached=E2=80=99 by one to >> =E2=80=98open-connection-for-uri/maybe=E2=80=99. Are we still using cac= hed >> connections? > > At least on that commit, open-connection-for-uri/maybe calls > open-connection-for-uri/cached, so yes, still using cached connections. OK. >> Commit f50f5751fff4cfc6d5abba9681054569694b7a5c no longer passes the >> #:port parameter to =E2=80=98http-fetch=E2=80=99. > > Yeah, that change is sort of fine if you're just looking at how the > port/connection is handled, but that area is being fixed up here, and > because closing the port is something that happens, it's better to also > pass the port in. OK. >> Commit 20c08a8a45d0f137ead7c05e720456b2aea44402 does other things but at >> first sight I=E2=80=99m not sure what the effect is. > > So, open-connection-for-uri/maybe is like > open-connection-for-uri/cached, but it catches a couple of exceptions > relating to not being able to connect to a substitute server, it also > remembers about showing the messages. > > The second commit here is changing that slightly, to not apply to > process-substitution, however I do think that code might have applied in > the past (as open-connection-for-uri/maybe was used I believe). But I > think you're right in saying there's probably some overlap between the > error handling here and done by with-networking. Alright. >> If you=E2=80=99re confident we can move forward to fix the bug, that=E2= =80=99s great >> (though we=E2=80=99ll need a good deal of testing), but I=E2=80=99d stil= l like to >> clarify these points later on. > > Well, the changes I'm suggesting here seem reasonable to me. As for > testing, checking things basically work is easy enough, but I don't > currently have many ideas for how to test for when fetching things > doesn't go to plan (which can of course happen). I=E2=80=99ll do some testing of v2 on my end and report back. Thanks for the explanations! Ludo=E2=80=99. From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 16 Mar 2021 21:31:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161593023616173 (code B ref 47160); Tue, 16 Mar 2021 21:31:01 +0000 Received: (at 47160) by debbugs.gnu.org; 16 Mar 2021 21:30:36 +0000 Received: from localhost ([127.0.0.1]:41222 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMHGr-0004Cj-0z for submit@debbugs.gnu.org; Tue, 16 Mar 2021 17:30:36 -0400 Received: from eggs.gnu.org ([209.51.188.92]:34796) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMHGm-0004CT-19 for 47160@debbugs.gnu.org; Tue, 16 Mar 2021 17:30:31 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:48375) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lMHGg-0001sF-Nq; Tue, 16 Mar 2021 17:30:22 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=44446 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lMHGf-0002LB-53; Tue, 16 Mar 2021 17:30:22 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210315151133.16282-1-mail@cbaines.net> <20210315161532.1716-1-mail@cbaines.net> Date: Tue, 16 Mar 2021 22:30:19 +0100 In-Reply-To: <20210315161532.1716-1-mail@cbaines.net> (Christopher Baines's message of "Mon, 15 Mar 2021 16:15:31 +0000") Message-ID: <87k0q67pn8.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Christopher Baines skribis: > In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called wit= hin > process-substitution was changed. As with-cached-connection actually inc= ludes > 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, > (call-)with-cached-connection is back, rebranded as > call-with-cached-connection-and-error-handling. > > * guix/scripts/substitute.scm (process-substitution): Retry once for some > errors when making HTTP requests to fetch substitutes. Please mention also the new procedure, and a =E2=80=9CFixes .=E2=80=9D line > +(define (call-with-cached-connection-and-error-handling 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) > + (=3D 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))) I find it marginally clearer to pass #:fresh? #t (as was done in the code removed in 7c85877fdf964694061e3192eac35723ebc047bf) than to rely on the closed-port side effect. I think it=E2=80=99s OK to remove =E2=80=98-and-error-handling=E2=80=99 bec= ause that doesn=E2=80=99t really tell much and because too many words obscure the message IMO, but that=E2=80=99s a detail. I also like the helper macro as was removed in 7c85877fdf964694061e3192eac35723ebc047bf. Apart from that LGTM. My limited testing suggests it=E2=80=99s working as intended. Thank you! Ludo=E2=80=99. From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 16 Mar 2021 23:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161593630925550 (code B ref 47160); Tue, 16 Mar 2021 23:12:02 +0000 Received: (at 47160) by debbugs.gnu.org; 16 Mar 2021 23:11:49 +0000 Received: from localhost ([127.0.0.1]:41419 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMIqr-0006e2-9I for submit@debbugs.gnu.org; Tue, 16 Mar 2021 19:11:49 -0400 Received: from mira.cbaines.net ([212.71.252.8]:33430) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMIqo-0006dr-Gb for 47160@debbugs.gnu.org; Tue, 16 Mar 2021 19:11:47 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 6D62927BC54; Tue, 16 Mar 2021 23:11:45 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id a3103721; Tue, 16 Mar 2021 23:11:44 +0000 (UTC) References: <20210315151133.16282-1-mail@cbaines.net> <20210315161532.1716-1-mail@cbaines.net> <87k0q67pn8.fsf_-_@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines In-reply-to: <87k0q67pn8.fsf_-_@gnu.org> Date: Tue, 16 Mar 2021 23:11:40 +0000 Message-ID: <87eegeelsj.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Christopher Baines skribis: > >> In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called wi= thin >> process-substitution was changed. As with-cached-connection actually in= cludes >> 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, >> (call-)with-cached-connection is back, rebranded as >> call-with-cached-connection-and-error-handling. >> >> * guix/scripts/substitute.scm (process-substitution): Retry once for some >> errors when making HTTP requests to fetch substitutes. > > Please mention also the new procedure, and a > =E2=80=9CFixes .=E2=80=9D line > >> +(define (call-with-cached-connection-and-error-handling 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 t= he >> + ;; meantime, we get EPIPE. In that case, open a fresh connecti= on >> + ;; 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) >> + (=3D 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-componen= t))) >> + (begin >> + (close-port port) ; close the port to get a fresh o= ne >> + (proc (get-port))) > > I find it marginally clearer to pass #:fresh? #t (as was done in > the code removed in 7c85877fdf964694061e3192eac35723ebc047bf) than to > rely on the closed-port side effect. > > I think it=E2=80=99s OK to remove =E2=80=98-and-error-handling=E2=80=99 b= ecause that doesn=E2=80=99t > really tell much and because too many words obscure the message IMO, but > that=E2=80=99s a detail. I also like the helper macro as was removed in > 7c85877fdf964694061e3192eac35723ebc047bf. Sure, I'll send a v3 set of patches shortly. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBROyxfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XdTOw/5AWZ05MSZ6gfR/2uvGkTXfiOnVGomLOZt c2zr3bTqvXtHXSJ9bjHG0ygRpJHjI+sGYX4JAxzLD4U1JwPRf1tg30vW8pTmpx+e KyyaGimYJc/YXQX3yDkPUeMYl5EJkrI4H3noHCkK+0RoyvKAhdiXtbijD2OPElbB kTkRxWW3UIl/37Z7mXnJmQM2K/E9vH1ICO8KkaMalIImLSkOQPpyV2MNeIfMwsLK YniB9JlhVjk3e+yv3ei8bGCU78+fbrbbEeELXbxi5ah06o0+IbG55TolM/62eWzA a9pB1RyCtwhGx0AHYyFq94Ef8eyrOu53V+36DyH2ql1MifxtdNgf9TrFbUSZQwcJ liYKGZskVbQWCECMqF54aHeLhF8uJuAK9auFCmNkrc8PrMeoxsohsHEtscFvIvF9 t5IjHQDaz9qEkvtIk6TWH6QeIZBYFuXa3bEpQzbC0+w1eFBbxoIpvAXO2dqAgxj9 fvPOz1tBijHoVvUnIE5dlUHvWL7QB8n/smF0M1XhyNHooQmmgzdA/0e73BCfQ0hy SGy44zoLJqtalDacQQg5dhDkUTo1mwEXuD5ITuJ6dd86MNZauRJwAZmOix2+6GE/ E9pUbaFz9hHyaQFsBbHnxnXbyViKlUGVQ6KrEaicvF0hKpL2yBuB1aAXxFPVb9fH xpXOYFFfb14= =Ccxw -----END PGP SIGNATURE----- --=-=-=-- From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH 2/2] scripts: substitute: Tweak error reporting in process-substitution. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 16 Mar 2021 23:47:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161593839128967 (code B ref 47160); Tue, 16 Mar 2021 23:47:02 +0000 Received: (at 47160) by debbugs.gnu.org; 16 Mar 2021 23:46:31 +0000 Received: from localhost ([127.0.0.1]:41463 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMJOR-0007X8-5M for submit@debbugs.gnu.org; Tue, 16 Mar 2021 19:46:31 -0400 Received: from mira.cbaines.net ([212.71.252.8]:33880) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMJOP-0007Wx-8s for 47160@debbugs.gnu.org; Tue, 16 Mar 2021 19:46:29 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id C635F27BC55 for <47160@debbugs.gnu.org>; Tue, 16 Mar 2021 23:46:28 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 6da438fe for <47160@debbugs.gnu.org>; Tue, 16 Mar 2021 23:46:28 +0000 (UTC) From: Christopher Baines Date: Tue, 16 Mar 2021 23:46:28 +0000 Message-Id: <20210316234628.24479-2-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210316234628.24479-1-mail@cbaines.net> References: <20210316234628.24479-1-mail@cbaines.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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. --- guix/scripts/substitute.scm | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm index 812f2999ab..2bbbafe204 100755 --- a/guix/scripts/substitute.scm +++ b/guix/scripts/substitute.scm @@ -448,14 +448,11 @@ the current output port." (warning (G_ "while fetching ~a: server is somewhat slow~%") (uri->string uri)) (warning (G_ "try `--no-substitutes' if the problem persists~%"))) - (call-with-connection-error-handling - uri - (lambda () - (with-cached-connection uri port - (http-fetch uri #:text? #f - #:port port - #:keep-alive? #t - #:buffered? #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))))) -- 2.30.1 From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH 1/2] scripts: substitute: Add back some error handling. References: <20210315151133.16282-1-mail@cbaines.net> In-Reply-To: <20210315151133.16282-1-mail@cbaines.net> Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 16 Mar 2021 23:47:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.161593839128973 (code B ref 47160); Tue, 16 Mar 2021 23:47:02 +0000 Received: (at 47160) by debbugs.gnu.org; 16 Mar 2021 23:46:31 +0000 Received: from localhost ([127.0.0.1]:41465 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMJOR-0007XA-EM for submit@debbugs.gnu.org; Tue, 16 Mar 2021 19:46:31 -0400 Received: from mira.cbaines.net ([212.71.252.8]:33878) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMJOP-0007Ww-7H for 47160@debbugs.gnu.org; Tue, 16 Mar 2021 19:46:29 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id BD80A27BC54 for <47160@debbugs.gnu.org>; Tue, 16 Mar 2021 23:46:28 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 6117e4c4 for <47160@debbugs.gnu.org>; Tue, 16 Mar 2021 23:46:28 +0000 (UTC) From: Christopher Baines Date: Tue, 16 Mar 2021 23:46:27 +0000 Message-Id: <20210316234628.24479-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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 . * 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. --- guix/scripts/substitute.scm | 39 ++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) 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 -- 2.30.1 From unknown Sat Aug 16 16:56:30 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Wed, 17 Mar 2021 20:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.16160123052082 (code B ref 47160); Wed, 17 Mar 2021 20:19:02 +0000 Received: (at 47160) by debbugs.gnu.org; 17 Mar 2021 20:18:25 +0000 Received: from localhost ([127.0.0.1]:44037 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMccb-0000XW-6c for submit@debbugs.gnu.org; Wed, 17 Mar 2021 16:18:25 -0400 Received: from eggs.gnu.org ([209.51.188.92]:43776) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMccY-0000XJ-Fs for 47160@debbugs.gnu.org; Wed, 17 Mar 2021 16:18:23 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:43091) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lMccT-0004Jm-8S; Wed, 17 Mar 2021 16:18:17 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=48650 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lMccS-00040C-7g; Wed, 17 Mar 2021 16:18:16 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210315151133.16282-1-mail@cbaines.net> <20210316234628.24479-1-mail@cbaines.net> Date: Wed, 17 Mar 2021 21:18:13 +0100 In-Reply-To: <20210316234628.24479-1-mail@cbaines.net> (Christopher Baines's message of "Tue, 16 Mar 2021 23:46:27 +0000") Message-ID: <8735wt1qm2.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Howdy! Christopher Baines skribis: > In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called wit= hin > 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 . > > * guix/scripts/substitute.scm (call-with-cached-connection): New procedur= e. > (with-cached-connection): New syntax rule. > (process-substitution): Retry once for some errors when making HTTP reque= sts > 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=E2=80=99. From unknown Sat Aug 16 16:56:30 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: Christopher Baines Subject: bug#47160: closed (Re: bug#47160: [PATCH] scripts: substitute: Add back some error handling.) Message-ID: References: <87zgz1cxv6.fsf@cbaines.net> <20210315151133.16282-1-mail@cbaines.net> X-Gnu-PR-Message: they-closed 47160 X-Gnu-PR-Package: guix-patches X-Gnu-PR-Keywords: patch Reply-To: 47160@debbugs.gnu.org Date: Wed, 17 Mar 2021 20:47:02 +0000 Content-Type: multipart/mixed; boundary="----------=_1616014022-5077-1" This is a multi-part message in MIME format... ------------=_1616014022-5077-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #47160: [PATCH] scripts: substitute: Add back some error handling. 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 47160@debbugs.gnu.org. --=20 47160: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D47160 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1616014022-5077-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 47160-done) by debbugs.gnu.org; 17 Mar 2021 20:46:11 +0000 Received: from localhost ([127.0.0.1]:44136 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMd3T-0001It-0T for submit@debbugs.gnu.org; Wed, 17 Mar 2021 16:46:11 -0400 Received: from mira.cbaines.net ([212.71.252.8]:53044) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lMd3O-0001Ih-SS for 47160-done@debbugs.gnu.org; Wed, 17 Mar 2021 16:46:08 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id D3F7027BCD1; Wed, 17 Mar 2021 20:46:05 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 73c70d83; Wed, 17 Mar 2021 20:46:05 +0000 (UTC) References: <20210315151133.16282-1-mail@cbaines.net> <20210316234628.24479-1-mail@cbaines.net> <8735wt1qm2.fsf_-_@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#47160: [PATCH] scripts: substitute: Add back some error handling. In-reply-to: <8735wt1qm2.fsf_-_@gnu.org> Date: Wed, 17 Mar 2021 20:46:05 +0000 Message-ID: <87zgz1cxv6.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 47160-done Cc: 47160-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Howdy! > > Christopher Baines skribis: > >> In f50f5751fff4cfc6d5abba9681054569694b7a5c, the way fetch was called wi= thin >> process-substitution was changed. As call-with-cached-connection actual= ly >> includes important error handling for the opening of a HTTP request, this >> change removed some error handling. This commit adds that back. >> >> Fixes . >> >> * guix/scripts/substitute.scm (call-with-cached-connection): New procedu= re. >> (with-cached-connection): New syntax rule. >> (process-substitution): Retry once for some errors when making HTTP requ= ests >> 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 i= s 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 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBSao1fFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XfRrA//R+PiZGPPWoZV8M1We/wQW8uQ7O3HwXPG c+klF38zgrbiFHreGriob3JwGdPTR+6NUm56Dne8aPRsLHdMdDWMcfAVsNDGtD5g mRh1vIC1mtd4hlXoiV8gSrsMX05anZ5HVXwqD7ZvzFfwQhQBI5N1XT2d2pcTY5PA NbGGNNmwAMzIlTmZOtOzx52aokX6Cffskw3//HoE/dJLEaUJ+61JqCB7X9svxJ22 jboq1uX+Q39HjL0nunE8BxthdQf45LvG53gJ/q5JK94o4fKyHUuvfukMEvvaffS8 bZnB74Zx2o9oQ3DFZ5ectQok3iEHV93GoTHhiKlzxVREKZ5Nm9n+LjfsO297/SYn 9bDT1XMWjFugxp2zxGTbob0ojw44DlO9toZK6LV7uqj2AQp0OSw0PLllLv7cWv7n jE/D2b+z8SqU6ytBe4LzmjEUVoDTQnbcP4ASiV2ixR7yXimc6o+2Mkh8lHTKeSwv vyoEDBjEB16mXm0SnjimbHvjlR205FoIGugW9o/Wl0rPE9P6RAESvr8C2+BzL1xP QvbJzrGo3oOVVHg6g1wu9YQ8A7vZbkGXZFTQSQjdZf2vdKorzkIvcaDYOPqmSdo6 QdgkWWIgPSYUzv3izEefLaaawsExHLVfyHXHiO74iV1t2YL8wgHb7RpvqVZYGpL9 AqvCigBmeM0= =t8H9 -----END PGP SIGNATURE----- --=-=-=-- ------------=_1616014022-5077-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 15 Mar 2021 15:11:39 +0000 Received: from localhost ([127.0.0.1]:36361 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLosd-0005pP-BS for submit@debbugs.gnu.org; Mon, 15 Mar 2021 11:11:39 -0400 Received: from lists.gnu.org ([209.51.188.17]:37518) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLosb-0005pG-Ex for submit@debbugs.gnu.org; Mon, 15 Mar 2021 11:11:37 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38552) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lLosb-000832-9h for guix-patches@gnu.org; Mon, 15 Mar 2021 11:11:37 -0400 Received: from mira.cbaines.net ([2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27]:38603) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lLosZ-0005u2-Cn for guix-patches@gnu.org; Mon, 15 Mar 2021 11:11:37 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 362DA27BC52 for ; Mon, 15 Mar 2021 15:11:33 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 8c7d25df for ; Mon, 15 Mar 2021 15:11:33 +0000 (UTC) From: Christopher Baines To: guix-patches@gnu.org Subject: [PATCH] scripts: substitute: Add back some error handling. Date: Mon, 15 Mar 2021 15:11:33 +0000 Message-Id: <20210315151133.16282-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.4 (-) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.4 (--) 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 ------------=_1616014022-5077-1--