Skip to content

5 - API (feat): Improved Errors#230

Merged
RobertRosca merged 5 commits into
mainfrom
feat/improved-api-errors
Jul 20, 2026
Merged

5 - API (feat): Improved Errors#230
RobertRosca merged 5 commits into
mainfrom
feat/improved-api-errors

Conversation

@RobertRosca

Copy link
Copy Markdown
Member

PR no. 5 of the big refactor.

This PR expands on the DWError exception, creating a few explicit Errors based on that which should be used throughout the application code whenever an error/failure needs to be raised to be shown to the client, and adds in code to handle the errors at the request 'edge' (rest, graphql) so that the right response is sent to the client.

Details

Previously the API used builtin/re-raised exceptions, which makes it hard to tell if an error is 'expected' (missing proposal, upstream service, permission denial, etc...) or not (random bug in our code/a dependency, divide by zero, unexpected None, etc...), which makes it harder to dispatch to the right handler.

ADR-001 explains this in more detail and says that there should be an easy way to figure out what error response to send to a client, and decides to do this by having some distinct error classes where this happens, with anything else mapped to generic 500 error. To keep things from going very out of control, mapping to a response should be done in one clear place (transport edge/handler).

Error Classes

Errors are in shared/errors.py. The errors are meant to be framework-agnostic/generic-ish, so they aren't subclasses of some specific FastAPI/Starlette/Strawberry/whatever class.

Instead all errors are based on the main error class, DWError, which has fields:

  • code: defaults to 500, so it's always defined
  • message
  • details
  • request_id - captured from structlog contextvars at raise time

Currently the following errors are defined:

  • InvalidInputError (400)
  • UnauthenticatedError (401)
  • ForbiddenError (403)
  • NotFoundError (404)
    • ProposalNotFoundError
  • UpstreamServiceError (502), e.g. MyMdC/Keycloak not available
  • DataUnavailableError (503), e.g. issue reading data files (runs.sqlite, hdf5)

Mapping

REST maps a DWError to {"message", "details", "request_id"} with exc.code via a FastAPI exception handler.

According to the ADR, GraphQL maps DWErrors as GraphQL errors with extensions: {code, request_id} so the frontend branches on code rather than message text.

This is a partial lie since that's not in this PR/branch and is included in a later branch in the stack that changes the resolvers, but that's the design.

Tests

test_errors.py covers:

  • code attributes
  • ProposalNotFoundError -> NotFoundError inheritance
  • details/request_id capture (including the unbound-contextvars case)
  • FastAPI handler mapping a raised DWError to the right status code and body

@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch from c12837f to bd0db2e Compare July 10, 2026 14:13
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from ea9ed06 to 4ca4f6e Compare July 10, 2026 14:13
@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch from bd0db2e to 934a15f Compare July 10, 2026 14:20
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from 4ca4f6e to 02a5f78 Compare July 10, 2026 14:20
@RobertRosca RobertRosca changed the title API (feat): Improved Errors 5 - API (feat): Improved Errors Jul 10, 2026

@CammilleCC CammilleCC left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment aside, LGTM. Error codes and exception names make sense.

Comment thread api/src/damnit_api/shared/errors.py Outdated
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from 02a5f78 to c60b40f Compare July 17, 2026 06:20
@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch 2 times, most recently from d3b6985 to 0a44707 Compare July 17, 2026 06:21
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from c60b40f to c80d085 Compare July 17, 2026 06:21
@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch from 0a44707 to 9288103 Compare July 17, 2026 08:40
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch 2 times, most recently from 0ddd29c to c9d9daf Compare July 17, 2026 09:09
@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch 2 times, most recently from d15702f to 7b7216a Compare July 17, 2026 09:11
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from c9d9daf to 4c5700c Compare July 17, 2026 09:11
@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch from 7b7216a to 92d115c Compare July 17, 2026 09:12
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch 2 times, most recently from bf4af8b to bce3ee7 Compare July 17, 2026 09:12
@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch 2 times, most recently from 22719a6 to a454529 Compare July 17, 2026 10:46
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch 2 times, most recently from 04f2042 to 951da6d Compare July 17, 2026 11:01
@RobertRosca
RobertRosca marked this pull request as ready for review July 17, 2026 12:00
@CammilleCC

CammilleCC commented Jul 17, 2026

Copy link
Copy Markdown
Member

LGTM after CI and the rename

@RobertRosca
RobertRosca force-pushed the feat/api-e2e-rec-tests branch from 3967e92 to 9dd4463 Compare July 20, 2026 09:25
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from 951da6d to 08d0c88 Compare July 20, 2026 09:26
Base automatically changed from feat/api-e2e-rec-tests to main July 20, 2026 09:28
@RobertRosca
RobertRosca force-pushed the feat/improved-api-errors branch from 08d0c88 to 349cfa2 Compare July 20, 2026 09:28
@RobertRosca
RobertRosca merged commit 8fb22e2 into main Jul 20, 2026
1 check passed
@RobertRosca
RobertRosca deleted the feat/improved-api-errors branch July 20, 2026 09:30
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.

2 participants