[BUG] Registry clients throw on transport errors, timeouts and malformed JSON instead of returning Failed #272

Summary

.squad/project.md (Guarantees) promises that integration clients never throw for expected failures and answer with ExternalOperationResult<T> instead. Several DockerHubClient and OciRegistryClient calls break that promise: a transport error or timeout that remains after the retries, or a response body that is not the JSON shape expected, escapes as an exception (HttpRequestException, TaskCanceledException, JsonException, InvalidOperationException, FormatException). Callers then cannot branch on the outcome; the error lands in a generic catch in the orchestrator, which records a raw exception message, and a timeout is even indistinguishable from cancellation in the paths that filter on OperationCanceledException.

Area

• docs/areas/registries.md (added in [Docs] Describe the current behavior by area #263): Failure model of integrations, Limits and errors
• Security area (.squad/project.md): Registry and Docker Hub calls (untrusted JSON); messages must keep honoring Secrets in configuration

Current behavior

Calls without any exception handling (every awaited SendAsync, ReadAsStreamAsync and JsonDocument.ParseAsync can throw out of them):

• src/DockerUpdateGuard/DockerHub/DockerHubClient.cs: GetCurrentUserAsync (:1018-1061), GetRepositoriesAsync (:1064-1124), GetRepositoryAsync (:1127-1181), GetTagAsync (:1184-1232), GetTagsAsync (:1235-1295). The Docker Hub login (RequestAccessTokenAsync :960-1001) and the token expiry parsing (ResolveTokenExpiry :161-184, JsonDocument.Parse / Convert.FromBase64String on a malformed JWT) run inside these calls and throw out of them as well.
• src/DockerUpdateGuard/Images/OciRegistryClient.cs: GetTagAsync (:1208-1244), GetTagsAsync (:1247-1309, including ReadTagNamesAsync where tagElement.GetString() at :727 throws InvalidOperationException for a non-string array element), GetImageConfigurationAsync (:1340-1398), and the token request GetBearerTokenAsync (:1114-1149) reached from all of them.

Calls that catch, but let timeouts escape: DockerHubClient.ResolveBaseImagesAsync (:1317), DockerHubClient.GetImageConfigurationAsync (:1376) and OciRegistryClient.ResolveBaseImagesAsync (:1327) use

catch (Exception exception) when (exception is not OperationCanceledException)

An HttpClient.Timeout (DockerHub:RequestTimeoutSeconds, src/DockerUpdateGuard/ServiceCollectionExtensions.cs:76, :82) surfaces as TaskCanceledException (an OperationCanceledException with an inner TimeoutException) while the caller's token is not cancelled, so a timeout escapes as if the scan had been cancelled.

Where the exceptions end up: in the orchestrators' generic handlers, for example src/DockerUpdateGuard/Images/ImageScanOrchestrator.cs:760-767 (scan run failed with exception.Message) and src/DockerUpdateGuard/Images/RuntimeContainerScanOrchestrator.cs:668-680 (snapshot assessment failed with exception.Message). TransientHttpRetryHandler retries HttpRequestException/TimeoutException (src/DockerUpdateGuard/Infrastructure/TransientHttpRetryHandler.cs:98-101) but rethrows the last one when retries are used up.

Reproduction: unit test with a stub HttpMessageHandler that (a) throws HttpRequestException, (b) never completes until the HttpClient.Timeout fires, or (c) returns 200 with body {"tags": [1, 2]} / not json. OciRegistryClient.GetTagsAsync("registry.example.com", "team/app") and DockerHubClient.GetTagAsync(...) throw instead of returning ExternalOperationStatus.Failed.

Contrast: DockerScoutVulnerabilityProvider.GetVulnerabilitiesAsync already does it right (src/DockerUpdateGuard/Vulnerabilities/DockerScoutVulnerabilityProvider.cs:440-449): it rethrows only OperationCanceledException when cancellationToken.IsCancellationRequested and maps everything else to failed.

Expected behavior

Decision (product manager): transport errors, timeouts and malformed JSON map to failed with a message; cancellation stays cancellation.

• Every public method of IDockerHubClient and IRegistryMetadataClient in DockerHubClient and OciRegistryClient returns ExternalOperationResult<T>.Failed(message) when, after the retries:
  • sending the request or reading the body raises an HttpRequestException or IOException;
  • the request times out (OperationCanceledException/TaskCanceledException while the caller's cancellationToken is not cancelled), with a message stating the timeout, for example "Tag lookup for 'registry.example.com/team/app:1.2' timed out after 30 seconds";
  • the body is not valid JSON or has an unexpected shape (JsonException, InvalidOperationException from JsonElement accessors, FormatException from token decoding).
• When the caller's cancellationToken is cancelled, the OperationCanceledException propagates unchanged.
• Messages name the operation and the image/repository/account, and the error kind (the exception message is acceptable for transport and JSON errors). They never contain the PAT, a bearer token, an Authorization header or basic credentials, and no query string of token-service URLs.
• Each mapped failure is logged once as a warning with the exception, via the existing source-generated logging classes (DockerHubClientLogging, and a logging class for OciRegistryClient instead of the ad-hoc LogWarning at :1329).
• Expected outcomes stay as they are: 404 → not found, other non-success statuses → failed with status code and body.

Acceptance criteria

• For each public method of both clients (GetCurrentUserAsync, GetRepositoriesAsync, GetRepositoryAsync, GetTagAsync, GetTagsAsync, ResolveBaseImagesAsync, GetImageConfigurationAsync): a transport error returns Failed, a timeout returns Failed with a timeout message, and malformed JSON returns Failed.
• A cancelled caller token still throws OperationCanceledException from every one of these methods.
• A malformed Docker Hub login response or a JWT whose payload is not valid base64/JSON yields Failed, not an exception.
• No failure message contains the configured PAT or a token (asserted in tests with a recognizable fake PAT).
• Unit tests (MSTest, stub HttpMessageHandler or NSubstitute, names per docs/UNIT_TESTS.md), for example OciRegistryClientGetTagsTransportErrorReturnsFailed, OciRegistryClientGetTagsNonStringTagReturnsFailed, OciRegistryClientGetImageConfigurationTimeoutReturnsFailed, OciRegistryClientGetTagCallerCancelledThrowsOperationCanceledException, DockerHubClientGetTagsMalformedJsonReturnsFailed, DockerHubClientGetRepositoriesTimeoutReturnsFailed, DockerHubClientResolveBaseImagesTimeoutReturnsFailed, DockerHubClientGetTagMalformedAccessTokenReturnsFailed. Timeouts are simulated without real waiting (for example a handler throwing TaskCanceledException with an inner TimeoutException while the caller token is not cancelled).
• Format, build, tests, Analyzer gate and Coverage gate from .squad/stack.md pass.
• The area document docs/areas/registries.md describes the new behavior and its Known gaps entry for this issue is removed (the Limits and errors paragraph "Some failures still surface as errors instead of outcomes …" goes away).

Implementation notes

• Recommendation: one private wrapper per client (for example ExecuteAsync<T>(string operation, string target, Func<Task<ExternalOperationResult<T>>> action, CancellationToken cancellationToken)) that contains the catch blocks, instead of copying try/catch into every method:

  catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; }
  catch (OperationCanceledException exception) { /* timeout -> Failed */ }
  catch (Exception exception) when (exception is HttpRequestException or IOException or JsonException or InvalidOperationException or FormatException) { /* -> Failed */ }

  Decide deliberately whether to catch all remaining exceptions too (as Docker Scout does); catching only the listed types keeps programming errors visible. Either way, ArgumentException from argument guards must keep throwing.
• GetTagsAsync of both clients pages through several requests: a failure on page n returns failed for the whole listing (no partial success), consistent with today's handling of non-success statuses.
• OciRegistryClient.AppendInspectedTagsAsync calls GetTagAsync per tag; once GetTagAsync returns failed instead of throwing, the tag is listed by name only (existing AppendTagResult behavior). Make sure a caller-cancelled token still aborts the listing.
• RequestTimeoutSeconds is fixed at startup (HttpClient timeout); read the value from the options or the HttpClient.Timeout for the message.
• Must not change: the ExternalOperationResult<T> outcome model and orchestrator branching (.squad/project.md, Guarantees); TransientHttpRetryHandler retry rules; secrets never in messages or logs.
• No configuration or migration impact.

Related

• F-048 (P7) — Scan-resilience defects (F-024 batch abort, F-025 transient finding loss) without regression test #19 (scan-resilience defects; orchestrator side of failures)
• PR [Docs] Describe the current behavior by area #263 (area documents)

Generated by Claude Code