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

Workerd fails to send webSocketAutoResponse for Durable Object Hibernate API in local development. #1009

Closed
safaalai opened this issue Aug 11, 2023 · 12 comments · Fixed by #1130 or #1309
Assignees
Labels
Durable Objects https://developers.cloudflare.com/durable-objects/

Comments

@safaalai
Copy link

safaalai commented Aug 11, 2023

Which Cloudflare product(s) does this pertain to?
Pages, Durable Objects, Wrangler dev

What version(s) of the tool(s) are you using?
3.5.0 Wrangler

What version of Node are you using?
18.16.1

What operating system are you using?
Mac OS 13.2.1

Describe the Bug
Workerd fails to send webSocketAutoResponse for Durable Object Hibernate API in local development.

Definitions
normal message = message processed by webSocketMessage handler which sends back an ack.
ping message = message processed by webSocketAutoResponse handler which sends back a pong

I start a pages project for local development using npx wrangler pages dev -- npm start. npm start kicks off my dev server which is proxied by wrangler. This project uses a pages function and a durable object to setup a webSocket connection using the Durable Object Hibernate API. The project uses webSocketAutoResponse to setup a 'pong' response for every 'ping' request.

I get the following results when I try to send messages from the client to the server:

  1. Single normal message ==> goes through and gets a response from the DO
  2. Normal messages on intervals ==> every message gets a response from the DO
  3. 'ping' auto messages on an interval by themselves ==> get no response
  4. single normal message followed by 'pings' on intervals ==> normal message gets response, 1st ping gets response, subsequent pings get no response
  5. normal & ping messages on intervals ==> normal & ping messages get a single response, all subsequent normal & ping messages get no response

Looking at the DO logs, any time auto messages are involved, (i.e. 'ping' message cases #3, #4, #5 above), I get:
workerd/jsg/util.c++:275:` error: e = kj/compat/http.c++:3421: failed: expected !currentlySending; another message send is already in progress stack: 10b9e595f 10b9e3338 10b9f7ccd 10b9f07fe 10b9fb371 10b9f7ccd 10b9f07fe 10b9fb371 10b9f7ccd 10b9f07fe 10b9fb371 10b0ce270; sentryErrorContext = jsgInternalError Client error Error: internal error { stack: Error: internal error, message: internal error }

@safaalai
Copy link
Author

Note, I was asked by @milan_cf on Discord to open the bug under workerd.

@kentonv
Copy link
Member

kentonv commented Aug 14, 2023

cc @MellowYarker (aka milan_cf on Discord)

@MellowYarker
Copy link
Contributor

@jqmmes is looking into this, we think we know the issue but need to get a repro. He's out for the week so I may pick up when I have a chance.

@safaalai
Copy link
Author

Thanks for looking into this. I'm working on creating a repo with code that produces the bug. Will post here tomorrow.

@safaalai
Copy link
Author

Here's a repo that reproduces the bug. Instructions are in the readme file.

https://github.com/safaalai/stencil-latest

@safaalai
Copy link
Author

safaalai commented Aug 26, 2023

@MellowYarker & @jqmmes mmes, any thoughts on when this bug may be looked into?

@MellowYarker
Copy link
Contributor

@safaalai we've identified a problem and have been working on a fix, sorry for not updating here. We found a race condition that broke a requirement of our library's websocket implementation; fix looked easy enough at a glance, but has turned out to be somewhat complicated. I expect the update will be in next weeks release, not exactly sure when wrangler updates its Workerd version but @jqmmes will comment here when it's ready 🙂

Thanks for your patience!

@safaalai
Copy link
Author

@MellowYarker any more news on this?

@MellowYarker
Copy link
Contributor

#1130 we actually discussed this yesterday, should be okay with this approach and will be getting reviewed this week.

@irvinebroque irvinebroque added the Durable Objects https://developers.cloudflare.com/durable-objects/ label Sep 29, 2023
@jqmmes jqmmes self-assigned this Oct 2, 2023
@jqmmes jqmmes reopened this Oct 2, 2023
@jqmmes
Copy link
Contributor

jqmmes commented Oct 2, 2023

A PR that fixes this issue was merged, but it's still waiting to release in wrangler.

@safaalai
Copy link
Author

safaalai commented Oct 14, 2023

@jqmmes Just checking in. Any ideas when this may be released in Wrangler?

@jqmmes jqmmes linked a pull request Oct 17, 2023 that will close this issue
@jqmmes
Copy link
Contributor

jqmmes commented Oct 17, 2023

@safaalai this fix was briefly reverted, but it's going to be merged once again soon. I don't have an ETA for the wrangler release yet, but I think it won't take long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Durable Objects https://developers.cloudflare.com/durable-objects/
Projects
None yet
5 participants