search

ueberdosis / hocuspocus

The CRDT Yjs WebSocket backend for conflict-free real-time collaboration in your app.
https://tiptap.dev/docs/hocuspocus/introduction
MIT License
1.21k stars 117 forks source link

Most hook promise rejections are unhandled #754

Open haines opened 9 months ago

haines commented 9 months ago

Description Most of the hook invocations are not guarded with .catch() or try-await-catch, so promise rejections are unhandled. This causes a server crash by default.

Steps to reproduce the bug Steps to reproduce the behavior:

  1. Return a rejected promise from a hook e.g. onStoreDocument
  2. Server process crashes due to unhandled rejection

Expected behavior Errors should be handled appropriately.

shincurry commented 1 month ago

I also encountered a similar issue where I used a database extension to fetch and store Yjs document data from MySQL.

MySQL sometimes throws an error (unable to "insert on duplicate", "upsert"), which will cause the entire Node.js server instance to crash.

timreinkeaxios commented 4 weeks ago

I ran into this myself. Note that there's an undocumented escape hatch: https://github.com/ueberdosis/hocuspocus/blob/edd7034aece9ba41dcb4efff1207fd06624f319d/packages/server/src/Hocuspocus.ts#L538-L540

If your error does not have a message field, it will be 'handled'. Of course, then you're left with an unsaved document and need to solve that somehow.

This is why the redis extension doesn't crash the server when it can't a lock for the document.

I'm not posting to discount the issue, just highlight something you might be able to use in the meantime. I agree there should be some documented way for hooks to fail without crashing. And, for onStoreDocument in particular, maybe there is some API / options for how to handle when a document fails to save (e.g. automatically scheduling a retry later).