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

add missing error handlers to some pieces of middleware #1521

Merged
merged 1 commit into from
Mar 25, 2025

Conversation

paulfitz
Copy link
Member

A user reported a session error causing Grist to crash: https://community.getgrist.com/t/a-template-for-self-hosting-grist-with-traefik-and-docker-compose/856/28

The underlying error could be some cookie-related muddle with domain configuration, but it should just result in a error being issued as a response to a request, and not bring down the server.

By forcing req.session to null artificially in the code and watching what breaks, I was able to replicate the symptoms. The middleware added here converts a server crash to an error response.

Copy link
Member

@dsagal dsagal left a comment

Choose a reason for hiding this comment

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

Looks sensible. Thanks!

A user reported a session error causing Grist to crash:
https://community.getgrist.com/t/a-template-for-self-hosting-grist-with-traefik-and-docker-compose/856/28

The underlying error could be some cookie-related muddle with
domain configuration, but it should just result in a error
being issued as a response to a request, and not bring down
the server.

By forcing `req.session` to null artificially in the code and watching
what breaks, I was able to replicate the symptoms. The middleware
added here converts a server crash to an error response.
@paulfitz paulfitz force-pushed the paulfitz/early-errors branch from f8c3005 to 08f228c Compare March 25, 2025 17:39
@paulfitz paulfitz merged commit cdffc31 into main Mar 25, 2025
12 checks passed
@paulfitz paulfitz deleted the paulfitz/early-errors branch March 25, 2025 18:13
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