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.

Sign up for GitHub

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

Explicitly log errors for Flight onError #1320

Merged
merged 5 commits into from
May 27, 2022
Merged

Explicitly log errors for Flight onError #1320

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a1e7181
wip: Explicitly log errors for Flight onError
jplhomer May 19, 2022
5d6a75e
Merge remote-tracking branch 'origin/v1.x-2022-07' into jl-server-errors
jplhomer May 25, 2022
5268fd6
Rename variables for clarity
jplhomer May 25, 2022
7a97dd5
Add changeset
jplhomer May 25, 2022
15871f4
Revert streaming thing
jplhomer May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions packages/hydrogen/src/entry-server.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ async function stream(
const {noScriptTemplate, bootstrapScripts, bootstrapModules} =
stripScriptsFromTemplate(template);

const {AppSSR, rscReadable} = buildAppSSR(
const {AppSSR, rscReadable, rscErrored} = buildAppSSR(
{
App,
log,
Expand Down Expand Up @@ -380,7 +380,7 @@ async function stream(
async function prepareForStreaming(flush: boolean) {
Object.assign(
responseOptions,
getResponseOptions(componentResponse, didError)
getResponseOptions(componentResponse, rscErrored ?? didError)
);

/**
Expand All @@ -406,9 +406,11 @@ async function stream(
responseOptions.headers.set(CONTENT_TYPE, HTML_CONTENT_TYPE);
writable.write(encoder.encode(DOCTYPE));

if (didError) {
if (rscErrored ?? didError) {
// This error was delayed until the headers were properly sent.
writable.write(encoder.encode(getErrorMarkup(didError)));
writable.write(
encoder.encode(getErrorMarkup((rscErrored ?? didError) as Error))
);
}

return true;
Expand Down Expand Up @@ -495,7 +497,12 @@ async function stream(
componentResponse.cacheControlHeader
);

writeHeadToServerResponse(response, componentResponse, log, didError);
writeHeadToServerResponse(
response,
componentResponse,
log,
rscErrored ?? didError
);

if (isRedirect(response)) {
// Return redirects early without further rendering/streaming
Expand All @@ -504,7 +511,10 @@ async function stream(

if (!componentResponse.canStream()) return;

startWritingHtmlToServerResponse(response, dev ? didError : undefined);
startWritingHtmlToServerResponse(
response,
dev ? rscErrored ?? didError : undefined
);

setTimeout(() => {
log.trace('node pipe response');
Expand All @@ -529,7 +539,12 @@ async function stream(
return;
}

writeHeadToServerResponse(response, componentResponse, log, didError);
writeHeadToServerResponse(
response,
componentResponse,
log,
rscErrored ?? didError
);

postRequestTasks(
'str',
Expand All @@ -547,7 +562,10 @@ async function stream(
return response.end(await componentResponse.customBody);
}

startWritingHtmlToServerResponse(response, dev ? didError : undefined);
startWritingHtmlToServerResponse(
response,
dev ? rscErrored ?? didError : undefined
);

bufferReadableStream(rscToScriptTagReadable.getReader()).then(
(scriptTags) => {
Expand Down Expand Up @@ -609,7 +627,11 @@ async function hydrate(
});

if (__WORKER__) {
const rscReadable = rscRenderToReadableStream(AppRSC);
const rscReadable = rscRenderToReadableStream(AppRSC, {
onError(e) {
log.error(e);
},
});

if (isStreamable && (await isStreamingSupported())) {
postRequestTasks('rsc', 200, request, componentResponse);
Expand All @@ -632,7 +654,11 @@ async function hydrate(
'@shopify/hydrogen/vendor/react-server-dom-vite/writer.node.server'
);

const streamer = rscWriter.renderToPipeableStream(AppRSC);
const streamer = rscWriter.renderToPipeableStream(AppRSC, {
onError(e: Error) {
log.error(e);
},
});
const stream = streamer.pipe(response) as Writable;

response.writeHead(200, 'ok', {
Expand Down Expand Up @@ -695,8 +721,17 @@ function buildAppSSR(
response,
});

const [rscReadableForFizz, rscReadableForFlight] =
rscRenderToReadableStream(AppRSC).tee();
let rscErrored;

const [rscReadableForFizz, rscReadableForFlight] = rscRenderToReadableStream(
AppRSC,
{
onError(e) {
rscErrored = e;
log.error(e);
},
}
).tee();

const rscResponse = createFromReadableStream(rscReadableForFizz);
const RscConsumer = () => rscResponse.readRoot();
Expand All @@ -721,7 +756,7 @@ function buildAppSSR(
</Html>
);

return {AppSSR, rscReadable: rscReadableForFlight};
return {AppSSR, rscReadable: rscReadableForFlight, rscErrored};
}

function PreloadQueries({
Expand Down
6 changes: 5 additions & 1 deletion packages/hydrogen/src/streaming.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import {createFromReadableStream as _createFromReadableStream} from '@shopify/hy
import type {Writable} from 'stream';

export const rscRenderToReadableStream = _rscRenderToReadableStream as (
App: JSX.Element
App: JSX.Element,
options?: {
onError?: (error: Error) => void;
}
) => ReadableStream<Uint8Array>;

export const createFromReadableStream = _createFromReadableStream as (
Expand Down Expand Up @@ -42,6 +45,7 @@ export const ssrRenderToReadableStream = _ssrRenderToReadableStream as (
) => Promise<ReadableStream<Uint8Array> & {allReady: Promise<void>}>;

export async function isStreamingSupported() {
return true;
jplhomer marked this conversation as resolved.
Show resolved Hide resolved
return Boolean(globalThis.Oxygen?.env?.HYDROGEN_ENABLE_WORKER_STREAMING);
}

Expand Down