change to using websocket hibernation API #32
Conversation
|
ready for review - we definitely shouldn't merge before cloudflare/cloudflare-docs#7872 and an open question when we want to replace the default example after that |
koeninger
changed the title
| // Create our session and add it to the sessions map. | ||
| let session = { limiterId, limiter, blockedMessages: [] }; | ||
| // attach limiterId to the webSocket so it survives hibernation | ||
| webSocket.serializeAttachment({ ...webSocket.deserializeAttachment(), limiterId: limiterId.toString() }); |
There was a problem hiding this comment.
Maybe a dumb question but why are we deserializing webSockets attachment here? Considering it was only just accepted as hibernatable I would think deserializeAttachment() would be null, right?
There was a problem hiding this comment.
If someone takes this example and adds more properties to the attachment, and this line was just overwriting whatever was already in the attachment with only the limiterId, then correctness would depend on whether they happened to add the new properties above or below this line.
Seems uglier but safer this way to always add another property to whatever was already in the attachment.
src/chat.mjs
Outdated
| webSocket.addEventListener("close", closeOrErrorHandler); | ||
| webSocket.addEventListener("error", closeOrErrorHandler); | ||
| async webSocketError(webSocket, error) { | ||
| this.webSocketClose(webSocket) |
There was a problem hiding this comment.
I don't know why but it feels weird to me that we could call this (maybe because it's a custom event in the runtime and I hadn't thought of it).
Either way, might be better to take the stuff inside of webSocketClose(), put it in another function, and then invoke that function from webSocketClose()/webSocketError(). I'm also not sure what would happen as it stands because code, reason, wasClean aren't being passed in. I guess they'd be treated as undefined, which is probably okay since only webSocket is referenced?
There was a problem hiding this comment.
Happy to move it to another function, but if there's some reason people can't manually call handlers, we should probably find out and document it
|
I get this error: cloudflare/workerd#736 Is the hibernation API ok to use? |
No description provided.