fix: add error handling to OpenAI-compatible serve endpoint

[markstur]

Misc PR

Type of PR

• Bug Fix
• New Feature
• Documentation
• Other

Description

• Link to Issue: Part of Create OpenAI API-compatible HTTP interface for mellea #521

Add proper exception handling to the chat completion endpoint in cli/serve/app.py to prevent unhandled exceptions from crashing the server. Returns appropriate HTTP status codes and error messages.

Changes:

• Wrap endpoint logic in try-except block with specific handlers
• AttributeError → 500 (missing output attributes)
• ValueError → 400 (validation/input errors)
• Exception → 500 (catch-all for unexpected errors)
• Add comprehensive test coverage (5 unit tests, 100% error path coverage)

Fixes ensure the server returns OpenAI-compatible error responses instead of crashing on exceptions.

Testing

• Tests added to the respective file if code was changed
• New code has 100% coverage if code as added
• Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

[github-actions]

The PR description has been updated. Please fill out the template for your PR to be reviewed.

[ajbozarth]

Claude found a few issues to look at:

Unused import (will fail ruff check) — cli/serve/app.py

Request is imported but never used:

from fastapi import FastAPI, Request  # Request is unused                                                                                                                                                                                

Test isolation — routes accumulate on the global app — test/cli/test_serve_errors.py

Each test calls app.add_api_route(...) on the shared module-level app object without cleanup. Routes pile up across test runs, which can cause 422 conflicts or ordering-dependent failures. Each test should use a fresh FastAPI()
instance or remove the route in teardown.

Nits

Duplicate error handlers — cli/serve/app.py

AttributeError and the catch-all Exception produce identical responses (500 server_error). The AttributeError branch can be dropped — Exception already covers it and the distinction adds nothing for callers.

response_model=None — minor type safety loss; Union[ChatCompletion, OpenAIErrorResponse] would preserve schema generation, though it won't change runtime behavior in this pattern.

[jakelorocco]

This seems reasonable to me. I have a few concerns that maybe aren't valid / might not need to be addressed (I don't know if I'm completely up to date on the purpose of this OpenAI server, so these might have already been discussed):

1. Are these errors helpful to the end user of the server? For instance, if the ValueError is raised by having some non-conforming field / input, that makes sense to me.
2. Will these errors leak anything that an end user might not want leaked? This may be out of scope, but I'm not sure if we have any expectations that the names of requirements, internal mellea functionality doesn't get leaked through these endpoints. Maybe thats on the end user to implement if they need that obfuscation?

[ajbozarth]

This LGTM now, but I id discuss @jakelorocco concerns with Claude and this was what we came to, probably worth addressing in a comment before merging if Claude is correct that m serve is not designed for prod, if it it we should address it (could be in a follow up imho):

Helpfulness — The ValueError → 400 path is genuinely useful since it reflects bad input the caller can fix. The 500 path is less useful to an end user but fine for a local dev server like m serve.

Information leakage — This is the more interesting concern. The current code passes str(e) directly into the error response body. For a 500, that could expose internal details like mellea class names, attribute paths, or module
structure. For a production-facing deployment that would be a real issue, but m serve is a local developer tool, so leaking implementation details to localhost is low risk. Worth the author explicitly acknowledging that — "this
is a local dev server, not intended for production exposure" — so the reviewer knows it was considered rather than overlooked.

[markstur]

rebased and resolved conflicts