Skip to content

Map NBP error statuses to specific errors with the status code - #16

Merged
milten89 merged 6 commits into
developfrom
fix/nbp-http-status-mapping
Oct 1, 2026
Merged

milten89 merged 6 commits into
developfrom
fix/nbp-http-status-mapping

Conversation

@milten89

@milten89 milten89 commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Fixes BACKLOG P0 item 3. Stacked on #14 (base fix/nbp-http-disposal). It will retarget to develop once #14 is merged.

Changes

Status mapping in GetNbpAsync, following ADR-0002:

Status Before After
400 UnknownError new BadRequestError with the NBP message, e.g. 400 BadRequest - Przekroczony limit 255 wyników / Maximum size of 255 data series has been exceeded
401 / 403 UnknownError UnauthorizedError
404 NotFoundError NotFoundError
429 RateLimitExceededError unchanged (with RetryAfter)
5xx UnknownError ServiceUnavailableError
other UnknownError UnknownError
  • Every HTTP error stores the status code in Metadata[OpenUrzednikError.StatusCodeMetadataKey] (as int).
  • Core API:
    • new BadRequestError / BadRequestException;
    • new public constant OpenUrzednikError.StatusCodeMetadataKey and a protected base constructor that takes a status code;
    • an optional int? statusCode = null parameter on NotFoundError, UnauthorizedError, ServiceUnavailableError, UnknownError and RateLimitExceededError. Callers' source still compiles, but binary compatibility breaks (the one-argument constructors are gone). That's acceptable before 1.0; put it in the release notes.
  • Error body: at most 500 characters are read, through a StreamReader, so a large proxy/HTML body is never buffered. If the body can't be read, the error is returned without the server message and a guarded Debug log is written.
  • Tests:
    • unit tests for every mapping, the server message, truncation and an unreadable body;
    • WireMock tests for 400/401/403/404/429/5xx, using 400 and 404 bodies captured from the real API (Nbp/Fixtures/error-*.txt). The real Content-Type is text/plain; charset=utf-8, checked with curl -D -.
  • NbpFixtures.cs and the fixtures csproj item match Fix NBP table B rates failing against the real API #13, so the two merge cleanly. A small docs/BACKLOG.md conflict is expected; resolve it when rebasing.

Observed while capturing fixtures: the API's date-range limit is 367 days (400 … Limit of 367 days has been exceeded). The validators still enforce 93 days, which is backlog item 7.

Verification

  • dotnet build OpenUrzednik.slnx: 0 errors (only the existing NU1902 warning).
  • dotnet test -f net10.0: Core 79/79, Nbp 480/480, IntegrationTests 42 passed / 5 skipped (manual tests).
  • dotnet format --verify-no-changes: clean.
  • reviewer agent: no blocking findings. I fixed all of its should-fix points: the bounded read, the csproj conflict and the test assertions.

🤖 Generated with Claude Code

milten89 and others added 4 commits October 1, 2026 17:50
The request and response were never disposed. With
ResponseHeadersRead, a non-success response kept its connection
until GC. Both are now disposed on every path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follows the status mapping in ADR-0002:
- 400 -> new BadRequestError carrying the NBP plain-text message
  (e.g. "400 BadRequest - Przekroczony limit 255 wyników / ..."),
  truncated to 500 characters.
- 401/403 -> UnauthorizedError.
- 5xx -> ServiceUnavailableError (previously UnknownError).
- Every HTTP error stores the status code in Metadata under
  OpenUrzednikError.StatusCodeMetadataKey.

Adds BadRequestError/BadRequestException to Core, an optional
statusCode parameter to the HTTP-related errors, and WireMock tests
for 400/401/403/404/429/5xx using bodies captured from the real API.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Read at most 500 characters of an error body through a StreamReader
instead of buffering the whole body, and log a guarded Debug message
when it can't be read. Assert exact status codes in the Core tests,
check truncation properly, and use one fixtures glob so the csproj
matches the table B branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base automatically changed from fix/nbp-http-disposal to develop October 1, 2026 19:31
@milten89 milten89 self-assigned this Oct 1, 2026
The merge of develop kept both copies of SendAsync and mixed the old
and new GetNbpAsync bodies, so the branch no longer compiled. Restore
the branch's version of the file (develop only changed it through #14,
which this branch already contains) and leave only item 4 under P0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@milten89
milten89 merged commit e8a4c55 into develop Oct 1, 2026
8 checks passed
@milten89
milten89 deleted the fix/nbp-http-status-mapping branch October 1, 2026 19:56
milten89 added a commit that referenced this pull request Oct 1, 2026
develop now contains #13, #14, #16 and #17 as squash commits; the
result equals develop plus this PR's two commits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
milten89 added a commit that referenced this pull request Oct 1, 2026
- P1: drop item 11, which #22 fixed (the #23 merge put it back); add
  item 27, the unconfirmed gold publication hour.
- P2: items 12 and 13 describe what is left after #16-#18; item 26
  (WireMock error paths per client) goes with item 12.
- P3: item 20 is no longer a vulnerability after #12; item 21 notes
  what is documented; new items 24 (Microsoft.Testing.Platform, blocks
  Dependabot #11) and 25 (outdated GITHUB-SETUP.md). Item 22 points
  to the new prose skills.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
milten89 added a commit that referenced this pull request Oct 1, 2026
- P1: drop item 11, which #22 fixed (the #23 merge put it back); add
  item 27, the unconfirmed gold publication hour.
- P2: items 12 and 13 describe what is left after #16-#18; item 26
  (WireMock error paths per client) goes with item 12.
- P3: item 20 is no longer a vulnerability after #12; item 21 notes
  what is documented; new items 24 (Microsoft.Testing.Platform, blocks
  Dependabot #11) and 25 (outdated GITHUB-SETUP.md). Item 22 points
  to the new prose skills.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant