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

Lazily stream responses #396

Merged
merged 8 commits into from
Jun 13, 2023
Merged

Lazily stream responses #396

merged 8 commits into from
Jun 13, 2023

Conversation

jridgewell
Copy link
Contributor

@jridgewell jridgewell commented Jun 13, 2023

This reimplements our streaming code to pull data from the ReadableStream lazily. The old could would eagerly read and push all data in the for await (…) loop.

This is particularly important for long-running requests when the user's connection is slow or cancels the browser request. If the client is slow, we now respect the back-pressure and wait for a drain event. And if the user cancels the request, we detect the close event to stop pulling more data.

I chose not to reuse the consumeUint8ArrayReadableStream, for a few reasons:

  • It does not properly cleanup the bodyStream
  • The code consuming it need to worry about the response's state
    • I plan on reusing this code in Next.js, and having a helper that handles all the complexities of lazy streaming is much nicer.

@changeset-bot
Copy link

changeset-bot bot commented Jun 13, 2023

🦋 Changeset detected

Latest commit: 8ccfe46

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
edge-runtime Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Jun 13, 2023

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

Name Status Preview Comments Updated (UTC)
edge-runtime ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 13, 2023 7:45am

@Kikobeats
Copy link
Member

This is really great, thanks for it 🙂

@Kikobeats Kikobeats merged commit 3da9178 into vercel:main Jun 13, 2023
@github-actions github-actions bot mentioned this pull request Jun 13, 2023
@jridgewell jridgewell deleted the lazy-stream branch June 13, 2023 15:17
jridgewell added a commit to jridgewell/edge-runtime that referenced this pull request Jun 23, 2023
* Use a lazy stream implementation

* Add proper test for unhandled rejection in pull

* Use consumeUint8ArrayReadableStream

* Remove module method mocking

* Extract stream helper so it can be reused by Next

* Fix test

* Export method

* Create spicy-toys-teach.md

---------

Co-authored-by: Kiko Beats <[email protected]>
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.

2 participants