Skip to content

fix(api): POST /api/requests/manual was unreachable - #235

Open
JediBrooker wants to merge 1 commit into
markbeep:mainfrom
JediBrooker:fix/manual-route-order
Open

JediBrooker wants to merge 1 commit into
markbeep:mainfrom
JediBrooker:fix/manual-route-order

Conversation

@JediBrooker

Copy link
Copy Markdown

What

POST /api/requests/manual is unreachable. It is declared after POST /{asin_or_uuid}, and FastAPI matches routes in declaration order, so the catch-all claims it and treats manual as an ASIN.

Reproducing

Against main, with any valid API key:

POST /api/requests/manual
{"title": "An Indie Book", "author": "Jane Doe"}

-> HTTP 404  {"detail":"Book not found"}

and in the log:

Failed to fetch book details from Audible  asin=manual
2 validation errors for AudibleSingleResponse
product.release_date  Field required [input_value={'asin': 'manual'}]
product.title         Field required [input_value={'asin': 'manual'}]

It is hitting create_request with asin_or_uuid="manual" and asking Audible for a book called "manual".

The web UI is unaffected, because app/routers/pages/search/manual.py imports create_manual_request and calls the function directly rather than going over HTTP. So only the REST endpoint is broken, which is probably why it has not come up.

Same class of problem applies only to this one route. GET /manual is fine because there is no bare GET /{asin_or_uuid}, and DELETE /manual/{id} and PATCH /manual/{id}/downloaded are fine because they have more path segments than the catch-alls they sit near.

The change

Moves ManualRequest and the POST /manual handler above POST /{asin_or_uuid}, with a comment so it does not drift back. No behaviour changes beyond the route becoming reachable.

After:

POST /api/requests/manual   -> HTTP 201, row stored
POST /api/requests/<asin>   -> still routes to create_request

The diff looks larger than it is: it is the same block of code moved, not rewritten.

Checked

  • uv run basedpyright — clean
  • uv run ruff format --check app — clean
  • just test_format, just test_jinja — clean
  • uv run alembic check — no new operations (no schema change here)
  • Verified by hand with TestClient both before and after: 404 before, 201 and a stored ManualBookRequest after, with the catch-all still reaching create_request

Written with AI assistance; the reproduction above was run against unmodified main.

The route is declared after POST /{asin_or_uuid}, and FastAPI matches in
declaration order, so the catch-all claims it. Creating a manual request over
the API is treated as a request for a book with the ASIN "manual", which is
then looked up on Audible and answered with 404:

    Failed to fetch book details from Audible  asin=manual
    2 validation errors for AudibleSingleResponse
    product.release_date  Field required
    product.title         Field required

The web UI is unaffected because it imports create_manual_request and calls it
directly rather than going over HTTP, which is likely why this went unnoticed.

Moves the handler and its request model above the catch-all, with a note so it
does not drift back. POST /{asin_or_uuid} still routes to create_request.

This branch has not been deployed

No deployments
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