Return the context's error when it ends during a fetch request (closes #221)

retryingClient.get checked its context right after each request and
returned that request's error, so a deadline that passed while a 503 was
on its way came back as the 503. TestGetStopsWaitingWhenCanceled failed
on that now and then on a loaded host. When the context ends before a
retry, during the request or during the wait, get now returns an error
wrapping both the context's error and the last try's error.

A new test covers the context ending during the request, and
TestGetStopsWaitingWhenCanceled now covers it ending during the wait.
Both answer through a transport rather than a server, so neither depends
on how fast the 503 arrives.

Model: opus-5-5

internal/cli/fetch_test.go

@@ -897,23 +897,59 @@ func TestGetHonorsRetryAfter(t *testing.T) {
 	}
 }
 
-// TestGetStopsWaitingWhenCanceled gives get a server that always answers
-// 503 and an hour's wait before each retry, then lets the context expire
-// during that wait. get returning at all shows it stopped waiting.
+// instant503Transport answers every request with a 503 at once, so the
+// 503 is in hand before any deadline can pass.
+type instant503Transport struct{}
+
+func (instant503Transport) RoundTrip(*http.Request) (*http.Response, error) {
+	return &http.Response{
+		StatusCode: http.StatusServiceUnavailable,
+		Body:       http.NoBody,
+	}, nil
+}
+
+// TestGetStopsWaitingWhenCanceled answers every request with a 503 and
+// sets an hour's wait before each retry, then lets the context expire
+// during that wait. get returning at all shows it stopped waiting; it must
+// return the deadline error and keep the 503 with it.
 func TestGetStopsWaitingWhenCanceled(t *testing.T) {
 	t.Parallel()
 
-	server := httptest.NewServer(
-		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusServiceUnavailable)
-		}))
-	defer server.Close()
-
 	client := testClient()
+	client.client.Transport = instant503Transport{}
 	client.firstDelay = time.Hour
 
-	err := getNothing(client, server.URL, 50*time.Millisecond)
+	err := getNothing(client, "http://example.invalid/", 50*time.Millisecond)
 	require.ErrorIs(t, err, context.DeadlineExceeded)
+	require.ErrorIs(t, err, errHTTPStatus)
+}
+
+// late503Transport answers every request with a 503, but only once the
+// request's context has ended. A real server cannot stand in for it: the
+// client drops a response still on its way when the context ends.
+type late503Transport struct{}
+
+func (late503Transport) RoundTrip(req *http.Request) (*http.Response, error) {
+	<-req.Context().Done()
+
+	return &http.Response{
+		StatusCode: http.StatusServiceUnavailable,
+		Body:       http.NoBody,
+	}, nil
+}
+
+// TestGetStopsWhenContextEndsDuringRequest lets the context expire while
+// the first request is still waiting for its 503. get must return the
+// deadline error and keep the 503 with it.
+func TestGetStopsWhenContextEndsDuringRequest(t *testing.T) {
+	t.Parallel()
+
+	client := testClient()
+	client.client.Transport = late503Transport{}
+
+	err := getNothing(client, "http://example.invalid/", 50*time.Millisecond)
+	require.ErrorIs(t, err, context.DeadlineExceeded)
+	require.ErrorIs(t, err, errHTTPStatus)
 }
 
 // TestDownloadFileRetriesToSuccess serves a file that fails twice before