Repository navigation
fix(api): complete the error mapping and test it through HTTP - #23
Merged
Merged
Conversation
The domain already returns link_invalid, system_record, self_deactivation and not_inactive, which the table did not list, so they would have become 500 responses. account_inactive maps to 401 (during a session); the sign-in endpoint returns its 403 explicitly.
API-12 says a 500 carries only the generic message and the details go to the logs. Returning the unmapped code and its message in a 500 exposed them and logged nothing; throwing lets the exception handler log it and answer with unexpected_error.
ADR-0003 asks endpoints to return typed results so the OpenAPI document can describe them; ProblemHttpResult lets an endpoint declare Results<Ok<T>, ProblemHttpResult>. The HttpContext parameter was left unused once ASP.NET Core became the source of traceId, so it is gone.
Issue #6 asked for an integration test of the documented error JSON. The first attempt mapped test-only endpoints only when the app was an IEndpointRouteBuilder, but a startup filter receives a plain ApplicationBuilder, so nothing was mapped. With UseRouting and UseEndpoints the endpoints run after UseExceptionHandler, which also covers the exception handler wiring that no test exercised. No Red: these cover wiring of existing behavior. Unregistering the exception handler makes both 500 tests fail. The unit tests they replace (exact JSON, traceId, the handler in isolation) are removed.
- API-15 records the verbatim message of validation_failed, which only appeared in an example. - The architecture guide describes ToProblem, the code table, account_inactive (401 by default, 403 at sign-in) and how a handler turns a field-less value object error into a field error. - The testing guide gains the startup-filter and DefaultHttpContext gotchas; the TEMPORARY exit-code notes are gone, since no project has that line any more. - Rule 3 of AGENTS.md and the git workflow now cover pull request bodies, because a squash merge copies them into main.
MendesMat
enabled auto-merge (squash)
September 28, 2026 05:04
5 tasks done
MendesMat
added a commit
that referenced
this pull request
Sep 28, 2026
…output check script (#25) ## What Documents how an issue is worked on with agents after the lessons of issue #6, and adds `check.ps1`, a single command that formats, builds and tests with short output. ## Why Issue #6 showed three problems: - **Cost.** One long conversation carried the survey, 13 TDD cycles and the review; repeated `ENDOFLINE` listings and full test logs filled it, and every line is paid again on each reply. - **Improvised decisions.** Where the plan was vague, the build step filled the gap by guessing, and one guess contradicted API-12. - **Late review.** The review happened after the merge, so its fixes needed a second pull request (#23). ## What changes - **Three sessions per issue** (`docs/agents/workflows/implement-a-feature.md`, section *Sessions* and new step 9): - plan, ending with a *plan comment* on the issue: decisions, new messages, and the test list with the expected result of each test; - build, which follows that comment instead of repeating the survey; - review in a new conversation, before the owner merges. - **Owner's guide** (`docs/agents/trabalhando-com-agentes.md`, in Portuguese): which model and effort to use in each session, what to paste to start each one, when to use pair or autonomous mode, when to switch models, and how to watch the quota. - **`check.ps1`** (`backend/ControlService`): runs `dotnet format`, then the build, then the tests. It prints only problems and summaries, and keeps the whole test output when a test fails, as the evidence of a Red. It replaces the separate commands in `AGENTS.md`, the testing guide, the git workflow and the backend README. - **`AGENTS.md`**: asks for short tool output, and treats a failed approach from the approved plan as a stop point. - **TDD workflow**: commit before each pause and show it in the report. - **Communication guide**: the plan comment is part of the normal flow, so it does not need a separate confirmation. ## How to test ```bash powershell.exe -NoProfile -File backend/ControlService/check.ps1 ``` Checked on three runs: - the whole solution: 137/137, exit 0; - a filtered run: 9/9, exit 0; - a throwaway failing test written with LF line endings: the endings were fixed silently, the failure was printed in full, and the exit code was 2. The relative links of every changed document resolve. ## Checklist - [x] Developed test-first (ADR-0033): not applicable, documentation and a tooling script, checked by the runs above - [x] `dotnet test --solution ControlService.slnx` passes locally - [x] Documentation updated (`AGENTS.md`, `docs/agents/`, backend README) - [x] Rule IDs covered or changed: none - [x] User-facing messages: none
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #6 (follow-ups found in the review after #22 was merged)
What
Fixes the gaps between the error mapping merged in #22 and the documentation, adds the real HTTP integration tests the issue asked for, adds the missing
IQueryHandler, and updates the agent guides.Why
link_invalid,system_record,self_deactivationandnot_inactive; the table did not list them, so they would have become 500 responses.account_inactivemaps to 401 (during a session); the sign-in endpoint returns its 403 explicitly.unexpected_error(API-14).error.ToProblem()returnsProblemHttpResult, so endpoints can declareResults<Ok<T>, ProblemHttpResult>for the OpenAPI document. Its unusedHttpContextparameter is gone.IStartupFilterreceives a plainApplicationBuilder, not anIEndpointRouteBuilder, so the test endpoints were never mapped. WithUseRouting+UseEndpoints,ErrorResponseTestsnow checks through the whole pipeline the documented validation JSON, the generic 500 for an exception and for an unmapped code. That also covers the exception handler wiring inProgram.cs, which no test exercised before.IQueryHandler<TQuery, TResponse>was in the scope of Application and API building blocks: handlers, validation and error responses #6 and was missed.validation_failedmessage, which only appeared in an example); the architecture guide describesToProblem, the code table,account_inactiveand how a handler turns a field-less value object error into a field error; the testing guide gains the startup filter andDefaultHttpContextgotchas and loses the staleTEMPORARYnotes; rule 3 ofAGENTS.mdand the git workflow now cover pull request bodies.How to test
dotnet test --solution ControlService.slnx137/137 pass, build with 0 warnings,
dotnet format --verify-no-changesclean.Checklist
ErrorResponseTestshad no Red: it covers wiring of existing behavior; unregistering the exception handler makes both 500 tests fail.IQueryHandleris an interface with no behavior.dotnet test --solution ControlService.slnxpasses locallydocs/api/conventions.md,docs/agents/,AGENTS.md)