Skip to content

fix: include request URL in NetworkException#31

Merged
rubenhensen merged 1 commit into
mainfrom
fix/network-exception-url
May 28, 2026
Merged

fix: include request URL in NetworkException#31
rubenhensen merged 1 commit into
mainfrom
fix/network-exception-url

Conversation

@dobby-coder

@dobby-coder dobby-coder Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the request URL to NetworkException so callers wrapping this SDK can tell PKG vs Cryptify failures (and which Cryptify phase) apart without spelunking through stack traces.

  • NetworkException gains a Url property; constructor signature is now (int statusCode, string body, string url).
  • Message format updated to HTTP {status} at {url}: {body} (e.g. HTTP 401 at https://pkg.postguard.eu/v2/irma/sign/key: Unauthorized).
  • Both EnsureSuccessAsync helpers (PkgClient, CryptifyClient) read response.RequestMessage?.RequestUri — set automatically by HttpClient — and pass it through. No per-call-site plumbing needed; all five failure sites (FetchMpkJsonAsync, FetchSigningKeysAsync, InitUploadAsync, StoreChunkAsync, FinalizeUploadAsync) gain context for free.
  • Adds NetworkExceptionTests (3 xUnit cases) covering the new property, message formatting, and an empty-body sanity case.

Closes #26

Test plan

  • dotnet build — net8.0 + net10.0, no warnings
  • dotnet test — 25/25 pass on net10.0 (net8.0 runtime not installed in this workspace; CI will exercise it)
  • CI green on the PR

NetworkException previously surfaced only the status code and response
body, so callers wrapping the SDK could not tell whether a 401 came
from PKG (MPK / signing-key fetch) or one of the three Cryptify upload
phases without digging through stack traces. Add a Url property,
include the URL in the message, and propagate it from both
EnsureSuccessAsync helpers via response.RequestMessage.RequestUri.

Closes #26
@dobby-coder dobby-coder Bot requested a review from rubenhensen May 27, 2026 23:36

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — would approve, but GitHub blocks self-approval on bot-authored PRs.

Implementation matches the suggested fix in #26 one-for-one: Url property added, both EnsureSuccessAsync helpers (src/Api/PkgClient.cs:71, src/Api/CryptifyClient.cs:131) read response.RequestMessage?.RequestUri with a <unknown> fallback, and message format becomes HTTP {status} at {url}: {body}.

Checked:

  • Correctness — all five failure sites (FetchMpkJsonAsync, FetchSigningKeysAsync, InitUploadAsync, StoreChunkAsync, FinalizeUploadAsync) pick up the URL transparently via HttpClient's auto-set RequestMessage.
  • Security — none of the constructed URLs carry auth tokens or query strings; bearer/cryptifytoken live in headers. The UUID path segment in the Cryptify upload URLs is operationally fine to surface in exception messages.
  • Null safety — ?. chain + ?? fallback covers a missing RequestMessage/RequestUri.
  • Tests — three new xUnit cases (NetworkExceptionTests) cover property population, message formatting, and empty body; 25/25 pass on net10.0 locally. CI net8.0 + net10.0 both pass.
  • API surface — constructor signature change is breaking in theory, but NetworkException is thrown by the SDK and not meant for consumer construction; pre-1.0 release-please flow handles this under fix:.
  • Build clean, no warnings.

Ready to merge.

@rubenhensen rubenhensen merged commit c33860b into main May 28, 2026
2 checks passed
@rubenhensen rubenhensen deleted the fix/network-exception-url branch May 28, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NetworkException loses endpoint URL — hard to tell PKG vs Cryptify failures apart

1 participant