Skip to content

Commit

Permalink
Fix streaming in Node.js fast path (#11058)
Browse files Browse the repository at this point in the history
* Fix streaming in Node.js fast path

* Create a new next if the iterator is not done

* Use a flag instead

* Update test

* Add new assertion

* Add explanation of the renderingComplete variable

* Remove flaky assertion
  • Loading branch information
matthewp authored May 16, 2024
1 parent 00420a7 commit 749a7ac
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/fluffy-pears-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fix streaming in Node.js fast path
22 changes: 16 additions & 6 deletions packages/astro/src/runtime/server/render/astro/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,23 @@ export async function renderToAsyncIterable(
let error: Error | null = null;
// The `next` is an object `{ promise, resolve, reject }` that we use to wait
// for chunks to be pushed into the buffer.
let next = promiseWithResolvers<void>();
let next: ReturnType<typeof promiseWithResolvers<void>> | null = null;
const buffer: Uint8Array[] = []; // []Uint8Array
let renderingComplete = false;

const iterator: AsyncIterator<Uint8Array> = {
async next() {
if (result.cancelled) return { done: true, value: undefined };

await next.promise;
if(next !== null) {
await next.promise;
}

// Only create a new promise if rendering is still ongoing. Otherwise
// there will be a dangling promises that breaks tests (probably not an actual app)
if(!renderingComplete) {
next = promiseWithResolvers();
}

// If an error occurs during rendering, throw the error as we cannot proceed.
if (error) {
Expand Down Expand Up @@ -276,8 +285,7 @@ export async function renderToAsyncIterable(
// Push the chunks into the buffer and resolve the promise so that next()
// will run.
buffer.push(bytes);
next.resolve();
next = promiseWithResolvers<void>();
next?.resolve();
}
},
};
Expand All @@ -286,12 +294,14 @@ export async function renderToAsyncIterable(
renderPromise
.then(() => {
// Once rendering is complete, calling resolve() allows the iterator to finish running.
next.resolve();
renderingComplete = true;
next?.resolve();
})
.catch((err) => {
// If an error occurs, save it in the scope so that we throw it when next() is called.
error = err;
next.resolve();
renderingComplete = true;
next?.resolve();
});

// This is the Iterator protocol, an object with a `Symbol.asyncIterator`
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/streaming.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('Streaming', () => {
let chunk = decoder.decode(bytes);
chunks.push(chunk);
}
assert.equal(chunks.length > 1, true);
assert.equal(chunks.length > 5, true);
});

it('Body of slots is chunked', async () => {
Expand Down

0 comments on commit 749a7ac

Please sign in to comment.