Skip to content

Podcasts router: route catch-all 500s through typed exceptions #1430

Open

Description

@lfnovo

Origin: cubic review on #1429 (comment)

Context

open_notebook/AGENTS.md says to raise typed exceptions from open_notebook.exceptions and let the global handlers in api/main.py map them to status codes, not bare HTTPException for domain errors. In api/routers/podcasts.py, five handlers already re-raise HTTPException and OpenNotebookError, but their final except Exception arm still raises a bare HTTPException(status_code=500, ...), and the original error is lost as the cause:

  • generate_podcast: "Failed to generate podcast"
  • job status: "Failed to fetch job status"
  • list episodes: "Failed to list podcast episodes"
  • retry episode: "Failed to retry episode"
  • delete episode: "Failed to delete episode"

Expected outcome

  • Each of the five fallbacks raises a typed exception (OpenNotebookError or a better-fitting subclass) with the same generic message, chained with from e. The global OpenNotebookError handler returns it as a 500.
  • The status code and the message the client sees stay exactly the same. They are deliberately generic: see tests/test_error_message_sanitization.py, and raw exception text must not reach the response.
  • The existing logger.error(...) stays.

Out of scope

  • The same pattern in other routers. There are about 110 catch-all status_code=500 sites, and a codebase-wide approach (for example, a global Exception handler that replaces the per-route catch-alls) needs its own design discussion.
  • Changing status codes or messages.

Acceptance criteria

  • No bare HTTPException(status_code=500, ...) remains in api/routers/podcasts.py.
  • Tests: for at least one of the five endpoints, an unexpected exception returns 500 with the same generic detail and without leaking the exception text.
  • pytest, ruff (check and format) and mypy green.

Activity

  1. MannXo commented on Oct 3, 2026

    @MannXo

    I'm interested in picking this up. Happy to discuss approach if needed.

  2. kamesprojects commented on Oct 3, 2026

    @kamesprojects

    Hey, I'd like to take this one if it's still free.

    I'd resolve it as described above, following the error-handling conventions in open_notebook/AGENTS.md. My plan is to swap the five HTTPException(status_code=500, ...) fallbacks in podcasts.py for OpenNotebookError with the same message and from e, so the global handler still returns the same 500 and nothing changes for the client. The logger.error calls stay as they are.

    I'll also add a test for each of the five endpoints to check that a random exception still gives the same generic 500 without leaking the error text.

    Happy to adjust anything if you had something different in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: podcastPodcast and audio featuresgood first issueGood for newcomersreadyApproved and ready to be worked on

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions