fix(frontend): handle non-JSON error responses in file fetches

[mkitti]

Fix #283 so that error responses do not result in JSON parsing errors of
that error response being displayed.

Before:

After:

Assisted by Gemini 3 Pro (Low) via Antigravity.

[krokicki]

I think it's useful to do some defensive programming here, but we should also try to understand why this response is being generated, and change it so that it generates JSON.

The Starlette exception handlers should be doing this, and it seems that is not working somehow.

    @app.exception_handler(StarletteHTTPException)
    async def http_exception_handler(request, exc):
        return JSONResponse({"error":str(exc.detail)}, status_code=exc.status_code)


    @app.exception_handler(RequestValidationError)
    async def validation_exception_handler(request, exc):
        return JSONResponse({"error":str(exc)}, status_code=400)

[allison-truhlar]

This frontend fix works for me as expected. I tested it with use_access_flags=true in the config.yaml. I also tested visiting files I didn't have permission to view when use_access_flags=false to ensure the permission error still showed up as expected.

I opened a PR against this branch to address Konrad's comment.

[neomorphic]

I think it's useful to do some defensive programming here, but we should also try to understand why this response is being generated, and change it so that it generates JSON.

The Starlette exception handlers should be doing this, and it seems that is not working somehow.

In reference to the comment, it looks like these two handlers do not catch all exceptions and so would not trigger on all occasions. We could add a general handler like this:

@app.exception_handler(Exception)
async def general_exception_handler(request, exc):
    logger.exception(f"Unhandled exception: {exc}")
    return JSONResponse({"error": str(exc)}, status_code=500)

This should catch everything and return a JSON response. The question is do we want all our 500 errors to be JSON responses? I think it will be fine, since the current 500 error response is a plain text document with

 "Internal Server Error"

and the new error response would be

{"error":"[Errno 1] Operation not permitted"}

Neither are very helpful to the user, but the second one does show better error messages when displayed in the application.

[neomorphic]

The frontend changes look good. There are probably many more cases (~25) where the response.json() should be caught to prevent this issue elsewhere. It looks like there are a number of instances later on in frontend/src/queries/fileQueries.ts that simply return an empty object if the json() method fails.

const body = await response.json().catch(() => ({}));

[allison-truhlar]

The frontend changes look good. There are probably many more cases (~25) where the response.json() should be caught to prevent this issue elsewhere. It looks like there are a number of instances later on in frontend/src/queries/fileQueries.ts that simply return an empty object if the json() method fails.

const body = await response.json().catch(() => ({}));

@neomorphic With the PR off this branch that addressed your above comment completed and merged, is this PR okay to merge into main now?

[neomorphic]

@neomorphic With the PR off this branch that addressed your above comment completed and merged, is this PR okay to merge into main now?

@allison-truhlar, Yeah, it should be fine.