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

fix: Handle WebSocket errors to avoid Node.js crashes #2522

Merged
merged 5 commits into from
Aug 24, 2022

Conversation

frol
Copy link
Contributor

@frol frol commented Aug 24, 2022

🎯 Changes

We caught the following error on our production:

image

I believe those malformed requests come from some vulnerability scanners. In either case, such requests should not crash the instance. During the investigation I found this discussion: websockets/ws#1354 (comment), and started to look into the way of handling the error. It seems there is no other way to inject this handler, so here is the PR. I have tested it already and it prevents the crash of the nodejs process.

✅ Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made. - I have no idea how to add the test here

jlalmes
jlalmes previously approved these changes Aug 24, 2022
sachinraja
sachinraja previously approved these changes Aug 24, 2022
@KATT KATT dismissed stale reviews from sachinraja and jlalmes via d0a6f8a August 24, 2022 15:15
@vercel
Copy link

vercel bot commented Aug 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
next-prisma-starter ✅ Ready (Inspect) Visit Preview Aug 24, 2022 at 3:19PM (UTC)
www ✅ Ready (Inspect) Visit Preview Aug 24, 2022 at 3:19PM (UTC)

@KATT
Copy link
Member

KATT commented Aug 24, 2022

cc @frol I've updated it to use opts.onError instead - lmk if you see any problems with that.

@KATT KATT merged commit 678f82f into main Aug 24, 2022
@KATT KATT deleted the fix/handle-websocket-errors branch August 24, 2022 16:19
@frol
Copy link
Contributor Author

frol commented Aug 25, 2022

That makes total sense, thanks for updating and merging the PR! 🎉

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

This pull request has been locked because it had no new activity for 30 days. If you think, this PR is still necessary, create a new one with the same branch. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants