TrustTheVote-Project/VersaEDM-Backend

Exceptions in API endpoints incorrectly causing HTTP 500 errors

Closed this issue · 1 comments

Raising ValueError in the EntityStore is clearly correct but it always propagates unhandled out of the event loop which means the server returns 500: Internal Server Error. Exceptions should be re-raised as a fastapi.HTTPException with an appropriate error code and message details.

This can be handled with try/except blocks in the router request handlers, but it gets verbose. A context manager might be cleaner. It's important to set the return code in the body of the handler though since different errors will require different HTTP codes.

diff --git a/src/electos/versadm/app/routers/party.py b/src/electos/versadm/app/routers/party.py
--- a/src/electos/versadm/app/routers/party.py
+++ b/src/electos/versadm/app/routers/party.py
@@ -32,13 +32,19 @@
     @request_handler
     def update_party(party_id: str, req: ApiRequest[Party]) -> str:
         # check that the record exists before updating
-        app_state.parties.get(party_id)
-        party_id = app_state.parties.put(req.data, overwrite=True)
+        try:
+            app_state.parties.get(party_id)
+            party_id = app_state.parties.put(req.data, overwrite=True)
+        except ValueError as ex:
+            raise HTTPException(400, str(ex))
         return party_id
 
     @router.delete('/parties/{party_id}')
     @request_handler
     def delete_party(party_id: str) -> bool:
-        return app_state.parties.delete(party_id)
+        try:
+            return app_state.parties.delete(party_id)
+        except ValueError as ex:
+            raise HTTPException(400, str(ex))
 
     return router

FastAPI provides for centralized customization of error handlers. I don't think that solves this issue though, because it customizes based on HTTP error code, and this issue is about deciding which HTTP error code to choose. As far as I can see that has to happen in the API endpoint handler, as that's the only point that can disambiguate between the various error cases without creating a dependency on HTTP error codes in lower-level modules.