Skip to content

Commit 5690932

Browse files
sebmarkbagesebmarkbage
authored
Add onErrorShell Callback (#23247)
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.
1 parent 0dedfcc commit 5690932

File tree

8 files changed

+137
-96
lines changed

8 files changed

+137
-96
lines changed

Diff for: ‎fixtures/fizz-ssr-browser/index.html

+22-14
Original file line numberDiff line numberDiff line change
@@ -20,29 +20,37 @@ <h1>Fizz Example</h1>
2020
<script src="../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js"></script>
2121
<script src="https://unpkg.com/babel-standalone@6/babel.js"></script>
2222
<script type="text/babel">
23-
let controller = new AbortController();
24-
let stream = ReactDOMServer.renderToReadableStream(
25-
<html>
26-
<body>Success</body>
27-
</html>,
28-
{
29-
signal: controller.signal,
23+
async function render() {
24+
let controller = new AbortController();
25+
let response;
26+
try {
27+
let stream = await ReactDOMServer.renderToReadableStream(
28+
<html>
29+
<body>Success</body>
30+
</html>,
31+
{
32+
signal: controller.signal,
33+
}
34+
);
35+
response = new Response(stream, {
36+
headers: {'Content-Type': 'text/html'},
37+
});
38+
} catch (x) {
39+
response = new Response('<!doctype><p>Error</p>', {
40+
status: 500,
41+
headers: {'Content-Type': 'text/html'},
42+
});
3043
}
31-
);
32-
let response = new Response(stream, {
33-
headers: {'Content-Type': 'text/html'},
34-
});
35-
display(response);
3644

37-
async function display(responseToDisplay) {
38-
let blob = await responseToDisplay.blob();
45+
let blob = await response.blob();
3946
let url = URL.createObjectURL(blob);
4047
let iframe = document.createElement('iframe');
4148
iframe.src = url;
4249
let container = document.getElementById('container');
4350
container.innerHTML = '';
4451
container.appendChild(iframe);
4552
}
53+
render();
4654
</script>
4755
</body>
4856
</html>

Diff for: ‎fixtures/ssr/server/render.js

+5
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ export default function render(url, res) {
2828
res.setHeader('Content-type', 'text/html');
2929
pipe(res);
3030
},
31+
onErrorShell(x) {
32+
// Something errored before we could complete the shell so we emit an alternative shell.
33+
res.statusCode = 500;
34+
res.send('<!doctype><p>Error</p>');
35+
},
3136
onError(x) {
3237
didError = true;
3338
console.error(x);

Diff for: ‎fixtures/ssr2/server/render.js

+5
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ module.exports = function render(url, res) {
4949
res.setHeader('Content-type', 'text/html');
5050
pipe(res);
5151
},
52+
onErrorShell(x) {
53+
// Something errored before we could complete the shell so we emit an alternative shell.
54+
res.statusCode = 500;
55+
res.send('<!doctype><p>Error</p>');
56+
},
5257
onError(x) {
5358
didError = true;
5459
console.error(x);

Diff for: ‎packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js

+34-42
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('ReactDOMFizzServer', () => {
5151

5252
// @gate experimental
5353
it('should call renderToReadableStream', async () => {
54-
const stream = ReactDOMFizzServer.renderToReadableStream(
54+
const stream = await ReactDOMFizzServer.renderToReadableStream(
5555
<div>hello world</div>,
5656
);
5757
const result = await readResult(stream);
@@ -60,7 +60,7 @@ describe('ReactDOMFizzServer', () => {
6060

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

7474
// @gate experimental
7575
it('should emit bootstrap script src at the end', async () => {
76-
const stream = ReactDOMFizzServer.renderToReadableStream(
76+
const stream = await ReactDOMFizzServer.renderToReadableStream(
7777
<div>hello world</div>,
7878
{
7979
bootstrapScriptContent: 'INIT();',
@@ -99,7 +99,7 @@ describe('ReactDOMFizzServer', () => {
9999
return 'Done';
100100
}
101101
let isComplete = false;
102-
const stream = ReactDOMFizzServer.renderToReadableStream(
102+
const stream = await ReactDOMFizzServer.renderToReadableStream(
103103
<div>
104104
<Suspense fallback="Loading">
105105
<Wait />
@@ -128,63 +128,55 @@ describe('ReactDOMFizzServer', () => {
128128
});
129129

130130
// @gate experimental
131-
it('should error the stream when an error is thrown at the root', async () => {
131+
it('should reject the promise when an error is thrown at the root', async () => {
132132
const reportedErrors = [];
133-
const stream = ReactDOMFizzServer.renderToReadableStream(
134-
<div>
135-
<Throw />
136-
</div>,
137-
{
138-
onError(x) {
139-
reportedErrors.push(x);
140-
},
141-
},
142-
);
143-
144133
let caughtError = null;
145-
let result = '';
146134
try {
147-
result = await readResult(stream);
148-
} catch (x) {
149-
caughtError = x;
135+
await ReactDOMFizzServer.renderToReadableStream(
136+
<div>
137+
<Throw />
138+
</div>,
139+
{
140+
onError(x) {
141+
reportedErrors.push(x);
142+
},
143+
},
144+
);
145+
} catch (error) {
146+
caughtError = error;
150147
}
151148
expect(caughtError).toBe(theError);
152-
expect(result).toBe('');
153149
expect(reportedErrors).toEqual([theError]);
154150
});
155151

156152
// @gate experimental
157-
it('should error the stream when an error is thrown inside a fallback', async () => {
153+
it('should reject the promise when an error is thrown inside a fallback', async () => {
158154
const reportedErrors = [];
159-
const stream = ReactDOMFizzServer.renderToReadableStream(
160-
<div>
161-
<Suspense fallback={<Throw />}>
162-
<InfiniteSuspend />
163-
</Suspense>
164-
</div>,
165-
{
166-
onError(x) {
167-
reportedErrors.push(x);
168-
},
169-
},
170-
);
171-
172155
let caughtError = null;
173-
let result = '';
174156
try {
175-
result = await readResult(stream);
176-
} catch (x) {
177-
caughtError = x;
157+
await ReactDOMFizzServer.renderToReadableStream(
158+
<div>
159+
<Suspense fallback={<Throw />}>
160+
<InfiniteSuspend />
161+
</Suspense>
162+
</div>,
163+
{
164+
onError(x) {
165+
reportedErrors.push(x);
166+
},
167+
},
168+
);
169+
} catch (error) {
170+
caughtError = error;
178171
}
179172
expect(caughtError).toBe(theError);
180-
expect(result).toBe('');
181173
expect(reportedErrors).toEqual([theError]);
182174
});
183175

184176
// @gate experimental
185177
it('should not error the stream when an error is thrown inside suspense boundary', async () => {
186178
const reportedErrors = [];
187-
const stream = ReactDOMFizzServer.renderToReadableStream(
179+
const stream = await ReactDOMFizzServer.renderToReadableStream(
188180
<div>
189181
<Suspense fallback={<div>Loading</div>}>
190182
<Throw />
@@ -205,7 +197,7 @@ describe('ReactDOMFizzServer', () => {
205197
// @gate experimental
206198
it('should be able to complete by aborting even if the promise never resolves', async () => {
207199
const controller = new AbortController();
208-
const stream = ReactDOMFizzServer.renderToReadableStream(
200+
const stream = await ReactDOMFizzServer.renderToReadableStream(
209201
<div>
210202
<Suspense fallback={<div>Loading</div>}>
211203
<InfiniteSuspend />

Diff for: ‎packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js

+15
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ describe('ReactDOMFizzServer', () => {
168168
// @gate experimental
169169
it('should error the stream when an error is thrown at the root', async () => {
170170
const reportedErrors = [];
171+
const reportedShellErrors = [];
171172
const {writable, output, completed} = getTestWritable();
172173
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
173174
<div>
@@ -178,6 +179,9 @@ describe('ReactDOMFizzServer', () => {
178179
onError(x) {
179180
reportedErrors.push(x);
180181
},
182+
onErrorShell(x) {
183+
reportedShellErrors.push(x);
184+
},
181185
},
182186
);
183187

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

195200
// @gate experimental
196201
it('should error the stream when an error is thrown inside a fallback', async () => {
197202
const reportedErrors = [];
203+
const reportedShellErrors = [];
198204
const {writable, output, completed} = getTestWritable();
199205
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
200206
<div>
@@ -207,6 +213,9 @@ describe('ReactDOMFizzServer', () => {
207213
onError(x) {
208214
reportedErrors.push(x);
209215
},
216+
onErrorShell(x) {
217+
reportedShellErrors.push(x);
218+
},
210219
},
211220
);
212221
pipe(writable);
@@ -216,11 +225,13 @@ describe('ReactDOMFizzServer', () => {
216225
expect(output.error).toBe(theError);
217226
expect(output.result).toBe('');
218227
expect(reportedErrors).toEqual([theError]);
228+
expect(reportedShellErrors).toEqual([theError]);
219229
});
220230

221231
// @gate experimental
222232
it('should not error the stream when an error is thrown inside suspense boundary', async () => {
223233
const reportedErrors = [];
234+
const reportedShellErrors = [];
224235
const {writable, output, completed} = getTestWritable();
225236
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
226237
<div>
@@ -233,6 +244,9 @@ describe('ReactDOMFizzServer', () => {
233244
onError(x) {
234245
reportedErrors.push(x);
235246
},
247+
onErrorShell(x) {
248+
reportedShellErrors.push(x);
249+
},
236250
},
237251
);
238252
pipe(writable);
@@ -243,6 +257,7 @@ describe('ReactDOMFizzServer', () => {
243257
expect(output.result).toContain('Loading');
244258
// While no error is reported to the stream, the error is reported to the callback.
245259
expect(reportedErrors).toEqual([theError]);
260+
expect(reportedShellErrors).toEqual([]);
246261
});
247262

248263
// @gate experimental

Diff for: ‎packages/react-dom/src/server/ReactDOMFizzServerBrowser.js

+45-40
Original file line numberDiff line numberDiff line change
@@ -32,54 +32,59 @@ type Options = {|
3232
bootstrapModules?: Array<string>,
3333
progressiveChunkSize?: number,
3434
signal?: AbortSignal,
35-
onCompleteShell?: () => void,
3635
onCompleteAll?: () => void,
3736
onError?: (error: mixed) => void,
3837
|};
3938

4039
function renderToReadableStream(
4140
children: ReactNodeList,
4241
options?: Options,
43-
): ReadableStream {
44-
const request = createRequest(
45-
children,
46-
createResponseState(
47-
options ? options.identifierPrefix : undefined,
48-
options ? options.nonce : undefined,
49-
options ? options.bootstrapScriptContent : undefined,
50-
options ? options.bootstrapScripts : undefined,
51-
options ? options.bootstrapModules : undefined,
52-
),
53-
createRootFormatContext(options ? options.namespaceURI : undefined),
54-
options ? options.progressiveChunkSize : undefined,
55-
options ? options.onError : undefined,
56-
options ? options.onCompleteAll : undefined,
57-
options ? options.onCompleteShell : undefined,
58-
);
59-
if (options && options.signal) {
60-
const signal = options.signal;
61-
const listener = () => {
62-
abort(request);
63-
signal.removeEventListener('abort', listener);
64-
};
65-
signal.addEventListener('abort', listener);
66-
}
67-
const stream = new ReadableStream({
68-
start(controller) {
69-
startWork(request);
70-
},
71-
pull(controller) {
72-
// Pull is called immediately even if the stream is not passed to anything.
73-
// That's buffering too early. We want to start buffering once the stream
74-
// is actually used by something so we can give it the best result possible
75-
// at that point.
76-
if (stream.locked) {
77-
startFlowing(request, controller);
78-
}
79-
},
80-
cancel(reason) {},
42+
): Promise<ReadableStream> {
43+
return new Promise((resolve, reject) => {
44+
function onCompleteShell() {
45+
const stream = new ReadableStream({
46+
pull(controller) {
47+
// Pull is called immediately even if the stream is not passed to anything.
48+
// That's buffering too early. We want to start buffering once the stream
49+
// is actually used by something so we can give it the best result possible
50+
// at that point.
51+
if (stream.locked) {
52+
startFlowing(request, controller);
53+
}
54+
},
55+
cancel(reason) {},
56+
});
57+
resolve(stream);
58+
}
59+
function onErrorShell(error: mixed) {
60+
reject(error);
61+
}
62+
const request = createRequest(
63+
children,
64+
createResponseState(
65+
options ? options.identifierPrefix : undefined,
66+
options ? options.nonce : undefined,
67+
options ? options.bootstrapScriptContent : undefined,
68+
options ? options.bootstrapScripts : undefined,
69+
options ? options.bootstrapModules : undefined,
70+
),
71+
createRootFormatContext(options ? options.namespaceURI : undefined),
72+
options ? options.progressiveChunkSize : undefined,
73+
options ? options.onError : undefined,
74+
options ? options.onCompleteAll : undefined,
75+
onCompleteShell,
76+
onErrorShell,
77+
);
78+
if (options && options.signal) {
79+
const signal = options.signal;
80+
const listener = () => {
81+
abort(request);
82+
signal.removeEventListener('abort', listener);
83+
};
84+
signal.addEventListener('abort', listener);
85+
}
86+
startWork(request);
8187
});
82-
return stream;
8388
}
8489

8590
export {renderToReadableStream, ReactVersion as version};

0 commit comments

Comments
 (0)