Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(chore): add error string to handle_snapshot #163

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

tabgok
Copy link
Contributor

@tabgok tabgok commented Feb 12, 2024

Prior to this change debugging the handle_snapshot function was difficult, because no error message beyond "INTERNAL SERVER ERROR" was returned. After this change, the exception's message is returned.

Prior to this change debugging the handle_snapshot function was
difficult, because no error message beyond "INTERNAL SERVER ERROR" was
returned.  After this change, the exception's message is returned.
@tabgok tabgok self-assigned this Feb 12, 2024
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this is a nice usability improvement. I think we can implement this generally in a middleware rather than just for this one endpoint.

Here's an example middleware that we already have

@middleware
async def session_token_middleware(request: Request, handler: _Handler) -> web.Response:
"""Extract session token from the request and store it in the request.
The token is retrieved from the headers or params of the request.
"""
token = _session_token(request)
request["session_token"] = token
return await handler(request)

Could you check to see if we could do that approach so we have this for all endpoints, @tabgok?

Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@Kyle-Verhoog Kyle-Verhoog merged commit 565ecd5 into master Feb 12, 2024
16 checks passed
@Kyle-Verhoog Kyle-Verhoog deleted the teague.bick/add-error-output branch February 12, 2024 22:33
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.

2 participants