Skip to content

Commit

Permalink
Add onErrorShell Callback (#23247)
Browse files Browse the repository at this point in the history
This indicates that an error has happened before the shell completed and
there's no point in emitting the result of this stream.

This is not quite the same as other fatal errors that can happen even
after streaming as started.

It's also not quite the same as onError before onCompleteShell because
onError can be called for an error inside a Suspense boundary before the
shell completes.

Implement shell error handling in Node SSR fixtures

Instead of hanging indefinitely.

Update Browser Fixture

Expose onErrorShell to the Node build

This API is not Promisified so it's just a separate callback instead.

Promisify the Browser Fizz API

It's now a Promise of a readable stream. The Promise resolves when the
shell completes. If the shell errors, the Promise is rejected.
  • Loading branch information
sebmarkbage authored Feb 9, 2022
1 parent 0dedfcc commit 5690932
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 96 deletions.
36 changes: 22 additions & 14 deletions fixtures/fizz-ssr-browser/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,37 @@ <h1>Fizz Example</h1>
<script src="../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js"></script>
<script src="https://unpkg.com/babel-standalone@6/babel.js"></script>
<script type="text/babel">
let controller = new AbortController();
let stream = ReactDOMServer.renderToReadableStream(
<html>
<body>Success</body>
</html>,
{
signal: controller.signal,
async function render() {
let controller = new AbortController();
let response;
try {
let stream = await ReactDOMServer.renderToReadableStream(
<html>
<body>Success</body>
</html>,
{
signal: controller.signal,
}
);
response = new Response(stream, {
headers: {'Content-Type': 'text/html'},
});
} catch (x) {
response = new Response('<!doctype><p>Error</p>', {
status: 500,
headers: {'Content-Type': 'text/html'},
});
}
);
let response = new Response(stream, {
headers: {'Content-Type': 'text/html'},
});
display(response);

async function display(responseToDisplay) {
let blob = await responseToDisplay.blob();
let blob = await response.blob();
let url = URL.createObjectURL(blob);
let iframe = document.createElement('iframe');
iframe.src = url;
let container = document.getElementById('container');
container.innerHTML = '';
container.appendChild(iframe);
}
render();
</script>
</body>
</html>
5 changes: 5 additions & 0 deletions fixtures/ssr/server/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export default function render(url, res) {
res.setHeader('Content-type', 'text/html');
pipe(res);
},
onErrorShell(x) {
// Something errored before we could complete the shell so we emit an alternative shell.
res.statusCode = 500;
res.send('<!doctype><p>Error</p>');
},
onError(x) {
didError = true;
console.error(x);
Expand Down
5 changes: 5 additions & 0 deletions fixtures/ssr2/server/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ module.exports = function render(url, res) {
res.setHeader('Content-type', 'text/html');
pipe(res);
},
onErrorShell(x) {
// Something errored before we could complete the shell so we emit an alternative shell.
res.statusCode = 500;
res.send('<!doctype><p>Error</p>');
},
onError(x) {
didError = true;
console.error(x);
Expand Down
76 changes: 34 additions & 42 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('ReactDOMFizzServer', () => {

// @gate experimental
it('should call renderToReadableStream', async () => {
const stream = ReactDOMFizzServer.renderToReadableStream(
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>hello world</div>,
);
const result = await readResult(stream);
Expand All @@ -60,7 +60,7 @@ describe('ReactDOMFizzServer', () => {

// @gate experimental
it('should emit DOCTYPE at the root of the document', async () => {
const stream = ReactDOMFizzServer.renderToReadableStream(
const stream = await ReactDOMFizzServer.renderToReadableStream(
<html>
<body>hello world</body>
</html>,
Expand All @@ -73,7 +73,7 @@ describe('ReactDOMFizzServer', () => {

// @gate experimental
it('should emit bootstrap script src at the end', async () => {
const stream = ReactDOMFizzServer.renderToReadableStream(
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>hello world</div>,
{
bootstrapScriptContent: 'INIT();',
Expand All @@ -99,7 +99,7 @@ describe('ReactDOMFizzServer', () => {
return 'Done';
}
let isComplete = false;
const stream = ReactDOMFizzServer.renderToReadableStream(
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback="Loading">
<Wait />
Expand Down Expand Up @@ -128,63 +128,55 @@ describe('ReactDOMFizzServer', () => {
});

// @gate experimental
it('should error the stream when an error is thrown at the root', async () => {
it('should reject the promise when an error is thrown at the root', async () => {
const reportedErrors = [];
const stream = ReactDOMFizzServer.renderToReadableStream(
<div>
<Throw />
</div>,
{
onError(x) {
reportedErrors.push(x);
},
},
);

let caughtError = null;
let result = '';
try {
result = await readResult(stream);
} catch (x) {
caughtError = x;
await ReactDOMFizzServer.renderToReadableStream(
<div>
<Throw />
</div>,
{
onError(x) {
reportedErrors.push(x);
},
},
);
} catch (error) {
caughtError = error;
}
expect(caughtError).toBe(theError);
expect(result).toBe('');
expect(reportedErrors).toEqual([theError]);
});

// @gate experimental
it('should error the stream when an error is thrown inside a fallback', async () => {
it('should reject the promise when an error is thrown inside a fallback', async () => {
const reportedErrors = [];
const stream = ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<Throw />}>
<InfiniteSuspend />
</Suspense>
</div>,
{
onError(x) {
reportedErrors.push(x);
},
},
);

let caughtError = null;
let result = '';
try {
result = await readResult(stream);
} catch (x) {
caughtError = x;
await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<Throw />}>
<InfiniteSuspend />
</Suspense>
</div>,
{
onError(x) {
reportedErrors.push(x);
},
},
);
} catch (error) {
caughtError = error;
}
expect(caughtError).toBe(theError);
expect(result).toBe('');
expect(reportedErrors).toEqual([theError]);
});

// @gate experimental
it('should not error the stream when an error is thrown inside suspense boundary', async () => {
const reportedErrors = [];
const stream = ReactDOMFizzServer.renderToReadableStream(
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<div>Loading</div>}>
<Throw />
Expand All @@ -205,7 +197,7 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should be able to complete by aborting even if the promise never resolves', async () => {
const controller = new AbortController();
const stream = ReactDOMFizzServer.renderToReadableStream(
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<div>Loading</div>}>
<InfiniteSuspend />
Expand Down
15 changes: 15 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should error the stream when an error is thrown at the root', async () => {
const reportedErrors = [];
const reportedShellErrors = [];
const {writable, output, completed} = getTestWritable();
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -178,6 +179,9 @@ describe('ReactDOMFizzServer', () => {
onError(x) {
reportedErrors.push(x);
},
onErrorShell(x) {
reportedShellErrors.push(x);
},
},
);

Expand All @@ -190,11 +194,13 @@ describe('ReactDOMFizzServer', () => {
expect(output.result).toBe('');
// This type of error is reported to the error callback too.
expect(reportedErrors).toEqual([theError]);
expect(reportedShellErrors).toEqual([theError]);
});

// @gate experimental
it('should error the stream when an error is thrown inside a fallback', async () => {
const reportedErrors = [];
const reportedShellErrors = [];
const {writable, output, completed} = getTestWritable();
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -207,6 +213,9 @@ describe('ReactDOMFizzServer', () => {
onError(x) {
reportedErrors.push(x);
},
onErrorShell(x) {
reportedShellErrors.push(x);
},
},
);
pipe(writable);
Expand All @@ -216,11 +225,13 @@ describe('ReactDOMFizzServer', () => {
expect(output.error).toBe(theError);
expect(output.result).toBe('');
expect(reportedErrors).toEqual([theError]);
expect(reportedShellErrors).toEqual([theError]);
});

// @gate experimental
it('should not error the stream when an error is thrown inside suspense boundary', async () => {
const reportedErrors = [];
const reportedShellErrors = [];
const {writable, output, completed} = getTestWritable();
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -233,6 +244,9 @@ describe('ReactDOMFizzServer', () => {
onError(x) {
reportedErrors.push(x);
},
onErrorShell(x) {
reportedShellErrors.push(x);
},
},
);
pipe(writable);
Expand All @@ -243,6 +257,7 @@ describe('ReactDOMFizzServer', () => {
expect(output.result).toContain('Loading');
// While no error is reported to the stream, the error is reported to the callback.
expect(reportedErrors).toEqual([theError]);
expect(reportedShellErrors).toEqual([]);
});

// @gate experimental
Expand Down
85 changes: 45 additions & 40 deletions packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,54 +32,59 @@ type Options = {|
bootstrapModules?: Array<string>,
progressiveChunkSize?: number,
signal?: AbortSignal,
onCompleteShell?: () => void,
onCompleteAll?: () => void,
onError?: (error: mixed) => void,
|};

function renderToReadableStream(
children: ReactNodeList,
options?: Options,
): ReadableStream {
const request = createRequest(
children,
createResponseState(
options ? options.identifierPrefix : undefined,
options ? options.nonce : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
),
createRootFormatContext(options ? options.namespaceURI : undefined),
options ? options.progressiveChunkSize : undefined,
options ? options.onError : undefined,
options ? options.onCompleteAll : undefined,
options ? options.onCompleteShell : undefined,
);
if (options && options.signal) {
const signal = options.signal;
const listener = () => {
abort(request);
signal.removeEventListener('abort', listener);
};
signal.addEventListener('abort', listener);
}
const stream = new ReadableStream({
start(controller) {
startWork(request);
},
pull(controller) {
// Pull is called immediately even if the stream is not passed to anything.
// That's buffering too early. We want to start buffering once the stream
// is actually used by something so we can give it the best result possible
// at that point.
if (stream.locked) {
startFlowing(request, controller);
}
},
cancel(reason) {},
): Promise<ReadableStream> {
return new Promise((resolve, reject) => {
function onCompleteShell() {
const stream = new ReadableStream({
pull(controller) {
// Pull is called immediately even if the stream is not passed to anything.
// That's buffering too early. We want to start buffering once the stream
// is actually used by something so we can give it the best result possible
// at that point.
if (stream.locked) {
startFlowing(request, controller);
}
},
cancel(reason) {},
});
resolve(stream);
}
function onErrorShell(error: mixed) {
reject(error);
}
const request = createRequest(
children,
createResponseState(
options ? options.identifierPrefix : undefined,
options ? options.nonce : undefined,
options ? options.bootstrapScriptContent : undefined,
options ? options.bootstrapScripts : undefined,
options ? options.bootstrapModules : undefined,
),
createRootFormatContext(options ? options.namespaceURI : undefined),
options ? options.progressiveChunkSize : undefined,
options ? options.onError : undefined,
options ? options.onCompleteAll : undefined,
onCompleteShell,
onErrorShell,
);
if (options && options.signal) {
const signal = options.signal;
const listener = () => {
abort(request);
signal.removeEventListener('abort', listener);
};
signal.addEventListener('abort', listener);
}
startWork(request);
});
return stream;
}

export {renderToReadableStream, ReactVersion as version};
Loading

0 comments on commit 5690932

Please sign in to comment.