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

New subscription-based adapters #5731

Draft
wants to merge 16 commits into
base: next
Choose a base branch
from

Conversation

domlen2003
Copy link

Closes #

🎯 Changes

  • implements a base utils for subscription-based adapters
  • implements the old ws adapter on top of the base utils
  • adds new cloudflare-durable-objects adapter

βœ… 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.

Copy link

vercel bot commented May 22, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
www ❌ Failed (Inspect) Jun 25, 2024 9:50pm

Copy link

vercel bot commented May 22, 2024

@domlen2003 is attempting to deploy a commit to the trpc Team on Vercel.

A member of the Team first needs to authorize it.

@domlen2003
Copy link
Author

Somehow it fails on client build, where I changed nothing?

@domlen2003
Copy link
Author

domlen2003 commented May 25, 2024

Okay i think there is actually some help needed with setting up the build for cloudflare worker types and e2e tests with a worker emulation.
I know there is a vitest plugin, but i don't know how to set that up.

@domlen2003
Copy link
Author

Also im currently running into a wall with DO I/O: Cannot perform I/O on behalf of a different Durable Object.
Any ideas?

@KATT
Copy link
Member

KATT commented May 25, 2024

This is very exciting - could you share some details on how we'd test this? I've never actually used the CF stuff here but I'm open to adding an adapter

Got limited time over the next week to play with this

@KATT
Copy link
Member

KATT commented May 25, 2024

Also as an FYI, we got #5713 pending which adds support for SSE and for subscriptions to be generator functions rather than our own observable thing. I don't know if CF has something to support SSE well but that'd be quite exciting

@domlen2003
Copy link
Author

domlen2003 commented May 25, 2024

Switching from the obervables would be a huge improvement imho, currently I recreate the subscription by sending it an empty dummy message. This is actually not optimal as it breaks the guaranty of the subscription message to be non null (if specified by the router).

Im fairly sure cloudflare doesn't support SSE on workers or durable objects. I don't know any of the reasoning behind it but probably they deemed it not useful enough since they made pretty good performance advancements with websocket hibernation.

@domlen2003
Copy link
Author

domlen2003 commented May 25, 2024

could you share some details on how we'd test this?

Well cloudflare has an (experimantal?) vitest integration tho so far i wasn't successfull in integrating it with the current test harness.

Also i have quite limited experience with rollup and struggle with setting up the 'cloudflare:workers' imports correctly.

In terms of which test are needed, I think we mainly need to focus on the hibernation/wakeup problem which also currently causes the I/O problems. Unfortunately I don't think there is a reliable way to forcefully hibernate a DO so we might need to simply rely on timeouts of +30s.

I think we could have an actual chance of cloudflare devs helping us out with these more specific problems, since their generally very "typing-affine" and sometimes take up projects like these in their free time (see workers-rs).

@BeBoRE
Copy link
Contributor

BeBoRE commented May 29, 2024

CloudFlare Workers do support SSE, you have to pass a ReadSteam inside of the response object that you return and you can write to this. CloudFlare limits workers to 50ms of CPU time, if you exceed this limit the worker gets terminated.

https://developers.cloudflare.com/workers/runtime-apis/streams/
https://blog.cloudflare.com/workers-ai-streaming/

Working demo:
https://github.com/BeBoRE/cloudflare-sse/
https://cloudflare-sse.bebore.workers.dev/

Events come in for about 2 hours using just an interval, so the request can be pretty long running.

@domlen2003
Copy link
Author

Okay thats interesting, but would we be able to hold it for longer in a Durable Object?

@BeBoRE
Copy link
Contributor

BeBoRE commented May 29, 2024

Those are voodoo, words to me :))

I have in the past attempted to implement subscriptions with CF workers, but I realised that it would be way to expensive if I didn't make use of hibernation, and I couldn't wrap my head around how that worked, so I gave up 😬.

@dinwwwh
Copy link

dinwwwh commented May 30, 2024

CF hibernation is the most powerful (it delivers messages in real-time) and cost-efficient method. Any other method will be limited in features or costly in terms of CPU time.

@domlen2003
Copy link
Author

Yes thats my opinion too, I would recommend focussing on the hibernation websocket API since this would be the most cost effective.
SSE on workers would probably be a nice to have but not as important.

@BeBoRE
Copy link
Contributor

BeBoRE commented May 30, 2024

From the code that I have read in #5713, SSE subscriptions should work out of the box with CloudFlare workers, however I couldn't get the repo to install to test it properly.

@domlen2003
Copy link
Author

It seems i have a working example rn. It actually survives hibernation and resusbcribes. Tho it's completely untested.

domlen2003 and others added 4 commits June 25, 2024 22:58
# Conflicts:
#	packages/server/src/adapters/subscribable/base.ts
#	packages/server/src/adapters/subscribable/ws.ts
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.

None yet

4 participants