@samitouri / QOS-React / commits / 85180b8cf8

[Fizz][Static] when aborting a prerender halt unfinished boundaries instead of erroring (#30732)

When we introduced prerendering for flight we modeled an abort of a flight prerender as having unfinished rows. This is similar to how postpone was already implemented when you postponed from "within" a prerender using React.unstable_postpone. However when aborting with a postponed instance every boundary would be eagerly marked for client rendering which is more akin to prerendering and then resuming with an aborted signal. The insight with the flight work was that it's not so much the postpone that describes the intended semantics but the abort combined with a prerender. So like in flight when you abort a prerender and enableHalt is enabled boundaries and the shell won't error for any reason. Fizz will still call onPostpone and onError according to the abort reason but the consuemr of the prerender should expect to resume it before trying to use it.

Josh Story committed Aug 20, 2024 at 13:30 UTC 85180b8cf84274795986c8f2c8473f8816db8b7b

5 files changed +424 -21

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

+106

@@ -7746,6 +7746,112 @@ describe('ReactDOMFizzServer', () => {

7746 );

7747 });

7748

7749 + // @gate enableHalt

7750 + it('can resume a prerender that was aborted', async () => {

7751 + const promise = new Promise(r => {});

7752 +

7753 + let prerendering = true;

7754 +

7755 + function Wait() {

7756 + if (prerendering) {

7757 + return React.use(promise);

7758 + } else {

7759 + return 'Hello';

7760 + }

7761 + }

7762 +

7763 + function App() {

7764 + return (

7765 + <div>

7766 + <Suspense fallback="Loading...">

7767 + <p>

7768 + <span>

7769 + <Suspense fallback="Loading again...">

7770 + <Wait />

7771 + </Suspense>

7772 + </span>

7773 + </p>

7774 + <p>

7775 + <span>

7776 + <Suspense fallback="Loading again too...">

7777 + <Wait />

7778 + </Suspense>

7779 + </span>

7780 + </p>

7781 + </Suspense>

7782 + </div>

7783 + );

7784 + }

7785 +

7786 + const controller = new AbortController();

7787 + const signal = controller.signal;

7788 +

7789 + const errors = [];

7790 + function onError(error) {

7791 + errors.push(error);

7792 + }

7793 + let pendingPrerender;

7794 + await act(() => {

7795 + pendingPrerender = ReactDOMFizzStatic.prerenderToNodeStream(<App />, {

7796 + signal,

7797 + onError,

7798 + });

7799 + });

7800 + controller.abort('boom');

7801 +

7802 + const prerendered = await pendingPrerender;

7803 +

7804 + expect(errors).toEqual(['boom', 'boom']);

7805 +

7806 + const preludeWritable = new Stream.PassThrough();

7807 + preludeWritable.setEncoding('utf8');

7808 + preludeWritable.on('data', chunk => {

7809 + writable.write(chunk);

7810 + });

7811 +

7812 + await act(() => {

7813 + prerendered.prelude.pipe(preludeWritable);

7814 + });

7815 +

7816 + expect(getVisibleChildren(container)).toEqual(

7817 + <div>

7818 + <p>

7819 + <span>Loading again...</span>

7820 + </p>

7821 + <p>

7822 + <span>Loading again too...</span>

7823 + </p>

7824 + </div>,

7825 + );

7826 +

7827 + prerendering = false;

7828 +

7829 + errors.length = 0;

7830 + const resumed = await ReactDOMFizzServer.resumeToPipeableStream(

7831 + <App />,

7832 + JSON.parse(JSON.stringify(prerendered.postponed)),

7833 + {

7834 + onError,

7835 + },

7836 + );

7837 +

7838 + await act(() => {

7839 + resumed.pipe(writable);

7840 + });

7841 +

7842 + expect(errors).toEqual([]);

7843 + expect(getVisibleChildren(container)).toEqual(

7844 + <div>

7845 + <p>

7846 + <span>Hello</span>

7847 + </p>

7848 + <p>

7849 + <span>Hello</span>

7850 + </p>

7851 + </div>,

7852 + );

7853 + });

7854 +

7855 // @gate enablePostpone

7856 it('does not call onError when you abort with a postpone instance during resume', async () => {

7857 let prerendering = true;

packages/react-dom/src/__tests__/ReactDOMFizzStatic-test.js

+52

@@ -454,4 +454,56 @@ describe('ReactDOMFizzStatic', () => {

454 });

455 expect(getVisibleChildren(container)).toEqual(undefined);

456 });

457 +

458 + // @gate enableHalt

459 + it('will halt a prerender when aborting with an error during a render', async () => {

460 + const controller = new AbortController();

461 + function App() {

462 + controller.abort('sync');

463 + return <div>hello world</div>;

464 + }

465 +

466 + const errors = [];

467 + const result = await ReactDOMFizzStatic.prerenderToNodeStream(<App />, {

468 + signal: controller.signal,

469 + onError(error) {

470 + errors.push(error);

471 + },

472 + });

473 + await act(async () => {

474 + result.prelude.pipe(writable);

475 + });

476 + expect(errors).toEqual(['sync']);

477 + expect(getVisibleChildren(container)).toEqual(undefined);

478 + });

479 +

480 + // @gate enableHalt

481 + it('will halt a prerender when aborting with an error in a microtask', async () => {

482 + const errors = [];

483 +

484 + const controller = new AbortController();

485 + function App() {

486 + React.use(

487 + new Promise(() => {

488 + Promise.resolve().then(() => {

489 + controller.abort('async');

490 + });

491 + }),

492 + );

493 + return <div>hello world</div>;

494 + }

495 +

496 + errors.length = 0;

497 + const result = await ReactDOMFizzStatic.prerenderToNodeStream(<App />, {

498 + signal: controller.signal,

499 + onError(error) {

500 + errors.push(error);

501 + },

502 + });

503 + await act(async () => {

504 + result.prelude.pipe(writable);

505 + });

506 + expect(errors).toEqual(['async']);

507 + expect(getVisibleChildren(container)).toEqual(undefined);

508 + });

509 });

packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js

+92 -9

@@ -307,7 +307,8 @@ describe('ReactDOMFizzStaticBrowser', () => {

307 });

308

309 // @gate experimental

310 - it('should reject if aborting before the shell is complete', async () => {

310 + // @gate !enableHalt

311 + it('should reject if aborting before the shell is complete and enableHalt is disabled', async () => {

312 const errors = [];

313 const controller = new AbortController();

314 const promise = serverAct(() =>

@@ -339,6 +340,42 @@ describe('ReactDOMFizzStaticBrowser', () => {

340 expect(errors).toEqual(['aborted for reasons']);

341 });

342

343 + // @gate enableHalt

344 + it('should resolve an empty prelude if aborting before the shell is complete', async () => {

345 + const errors = [];

346 + const controller = new AbortController();

347 + const promise = serverAct(() =>

348 + ReactDOMFizzStatic.prerender(

349 + <div>

350 + <InfiniteSuspend />

351 + </div>,

352 + {

353 + signal: controller.signal,

354 + onError(x) {

355 + errors.push(x.message);

356 + },

357 + },

358 + ),

359 + );

360 +

361 + await jest.runAllTimers();

362 +

363 + const theReason = new Error('aborted for reasons');

364 + controller.abort(theReason);

365 +

366 + let rejected = false;

367 + let prelude;

368 + try {

369 + ({prelude} = await promise);

370 + } catch (error) {

371 + rejected = true;

372 + }

373 + expect(rejected).toBe(false);

374 + expect(errors).toEqual(['aborted for reasons']);

375 + const content = await readContent(prelude);

376 + expect(content).toBe('');

377 + });

378 +

379 // @gate experimental

380 it('should be able to abort before something suspends', async () => {

381 const errors = [];

@@ -365,18 +402,26 @@ describe('ReactDOMFizzStaticBrowser', () => {

402 ),

403 );

404

368 - let caughtError = null;

369 - try {

370 - await streamPromise;

371 - } catch (error) {

372 - caughtError = error;

405 + if (gate(flags => flags.enableHalt)) {

406 + const {prelude} = await streamPromise;

407 + const content = await readContent(prelude);

408 + expect(errors).toEqual(['The operation was aborted.']);

409 + expect(content).toBe('');

410 + } else {

411 + let caughtError = null;

412 + try {

413 + await streamPromise;

414 + } catch (error) {

415 + caughtError = error;

416 + }

417 + expect(caughtError.message).toBe('The operation was aborted.');

418 + expect(errors).toEqual(['The operation was aborted.']);

419 }

374 - expect(caughtError.message).toBe('The operation was aborted.');

375 - expect(errors).toEqual(['The operation was aborted.']);

420 });

421

422 // @gate experimental

379 - it('should reject if passing an already aborted signal', async () => {

423 + // @gate !enableHalt

424 + it('should reject if passing an already aborted signal and enableHalt is disabled', async () => {

425 const errors = [];

426 const controller = new AbortController();

427 const theReason = new Error('aborted for reasons');

@@ -410,6 +455,44 @@ describe('ReactDOMFizzStaticBrowser', () => {

455 expect(errors).toEqual(['aborted for reasons']);

456 });

457

458 + // @gate enableHalt

459 + it('should resolve an empty prelude if passing an already aborted signal', async () => {

460 + const errors = [];

461 + const controller = new AbortController();

462 + const theReason = new Error('aborted for reasons');

463 + controller.abort(theReason);

464 +

465 + const promise = serverAct(() =>

466 + ReactDOMFizzStatic.prerender(

467 + <div>

468 + <Suspense fallback={<div>Loading</div>}>

469 + <InfiniteSuspend />

470 + </Suspense>

471 + </div>,

472 + {

473 + signal: controller.signal,

474 + onError(x) {

475 + errors.push(x.message);

476 + },

477 + },

478 + ),

479 + );

480 +

481 + // Technically we could still continue rendering the shell but currently the

482 + // semantics mean that we also abort any pending CPU work.

483 + let didThrow = false;

484 + let prelude;

485 + try {

486 + ({prelude} = await promise);

487 + } catch (error) {

488 + didThrow = true;

489 + }

490 + expect(didThrow).toBe(false);

491 + expect(errors).toEqual(['aborted for reasons']);

492 + const content = await readContent(prelude);

493 + expect(content).toBe('');

494 + });

495 +

496 // @gate experimental

497 it('supports custom abort reasons with a string', async () => {

498 const promise = new Promise(r => {});

packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js

+89 -9

@@ -212,7 +212,8 @@ describe('ReactDOMFizzStaticNode', () => {

212 });

213

214 // @gate experimental

215 - it('should reject if aborting before the shell is complete', async () => {

215 + // @gate !enableHalt

216 + it('should reject if aborting before the shell is complete and enableHalt is disabled', async () => {

217 const errors = [];

218 const controller = new AbortController();

219 const promise = ReactDOMFizzStatic.prerenderToNodeStream(

@@ -242,6 +243,40 @@ describe('ReactDOMFizzStaticNode', () => {

243 expect(errors).toEqual(['aborted for reasons']);

244 });

245

246 + // @gate enableHalt

247 + it('should resolve an empty shell if aborting before the shell is complete', async () => {

248 + const errors = [];

249 + const controller = new AbortController();

250 + const promise = ReactDOMFizzStatic.prerenderToNodeStream(

251 + <div>

252 + <InfiniteSuspend />

253 + </div>,

254 + {

255 + signal: controller.signal,

256 + onError(x) {

257 + errors.push(x.message);

258 + },

259 + },

260 + );

261 +

262 + await jest.runAllTimers();

263 +

264 + const theReason = new Error('aborted for reasons');

265 + controller.abort(theReason);

266 +

267 + let didThrow = false;

268 + let prelude;

269 + try {

270 + ({prelude} = await promise);

271 + } catch (error) {

272 + didThrow = true;

273 + }

274 + expect(didThrow).toBe(false);

275 + expect(errors).toEqual(['aborted for reasons']);

276 + const content = await readContent(prelude);

277 + expect(content).toBe('');

278 + });

279 +

280 // @gate experimental

281 it('should be able to abort before something suspends', async () => {

282 const errors = [];

@@ -266,18 +301,26 @@ describe('ReactDOMFizzStaticNode', () => {

301 },

302 );

303

269 - let caughtError = null;

270 - try {

271 - await streamPromise;

272 - } catch (error) {

273 - caughtError = error;

304 + if (gate(flags => flags.enableHalt)) {

305 + const {prelude} = await streamPromise;

306 + const content = await readContent(prelude);

307 + expect(errors).toEqual(['This operation was aborted']);

308 + expect(content).toBe('');

309 + } else {

310 + let caughtError = null;

311 + try {

312 + await streamPromise;

313 + } catch (error) {

314 + caughtError = error;

315 + }

316 + expect(caughtError.message).toBe('This operation was aborted');

317 + expect(errors).toEqual(['This operation was aborted']);

318 }

275 - expect(caughtError.message).toBe('This operation was aborted');

276 - expect(errors).toEqual(['This operation was aborted']);

319 });

320

321 // @gate experimental

280 - it('should reject if passing an already aborted signal', async () => {

322 + // @gate !enableHalt

323 + it('should reject if passing an already aborted signal and enableHalt is disabled', async () => {

324 const errors = [];

325 const controller = new AbortController();

326 const theReason = new Error('aborted for reasons');

@@ -309,6 +352,43 @@ describe('ReactDOMFizzStaticNode', () => {

352 expect(errors).toEqual(['aborted for reasons']);

353 });

354

355 + // @gate enableHalt

356 + it('should resolve with an empty prelude if passing an already aborted signal', async () => {

357 + const errors = [];

358 + const controller = new AbortController();

359 + const theReason = new Error('aborted for reasons');

360 + controller.abort(theReason);

361 +

362 + const promise = ReactDOMFizzStatic.prerenderToNodeStream(

363 + <div>

364 + <Suspense fallback={<div>Loading</div>}>

365 + <InfiniteSuspend />

366 + </Suspense>

367 + </div>,

368 + {

369 + signal: controller.signal,

370 + onError(x) {

371 + errors.push(x.message);

372 + },

373 + },

374 + );

375 +

376 + // Technically we could still continue rendering the shell but currently the

377 + // semantics mean that we also abort any pending CPU work.

378 +

379 + let didThrow = false;

380 + let prelude;

381 + try {

382 + ({prelude} = await promise);

383 + } catch (error) {

384 + didThrow = true;

385 + }

386 + expect(didThrow).toBe(false);

387 + expect(errors).toEqual(['aborted for reasons']);

388 + const content = await readContent(prelude);

389 + expect(content).toBe('');

390 + });

391 +

392 // @gate experimental

393 it('supports custom abort reasons with a string', async () => {

394 const promise = new Promise(r => {});

packages/react-server/src/ReactFizzServer.js

+85 -3

@@ -157,6 +157,7 @@ import {

157 enableSuspenseAvoidThisFallbackFizz,

158 enableCache,

159 enablePostpone,

160 + enableHalt,

161 enableRenderableContext,

162 enableRefAsProp,

163 disableDefaultPropsExceptForClasses,

@@ -3625,6 +3626,9 @@ function erroredTask(

3626 ) {

3627 // Report the error to a global handler.

3628 let errorDigest;

3629 + // We don't handle halts here because we only halt when prerendering and

3630 + // when prerendering we should be finishing tasks not erroring them when

3631 + // they halt or postpone

3632 if (

3633 enablePostpone &&

3634 typeof error === 'object' &&

@@ -3812,6 +3816,17 @@ function abortTask(task: Task, request: Request, error: mixed): void {

3816 logRecoverableError(request, fatal, errorInfo, null);

3817 fatalError(request, fatal, errorInfo, null);

3818 }

3819 + } else if (

3820 + enableHalt &&

3821 + request.trackedPostpones !== null &&

3822 + segment !== null

3823 + ) {

3824 + const trackedPostpones = request.trackedPostpones;

3825 + // We are aborting a prerender and must treat the shell as halted

3826 + // We log the error but we still resolve the prerender

3827 + logRecoverableError(request, error, errorInfo, null);

3828 + trackPostpone(request, trackedPostpones, task, segment);

3829 + finishedTask(request, null, segment);

3830 } else {

3831 logRecoverableError(request, error, errorInfo, null);

3832 fatalError(request, error, errorInfo, null);

@@ -3856,10 +3871,40 @@ function abortTask(task: Task, request: Request, error: mixed): void {

3871 }

3872 } else {

3873 boundary.pendingTasks--;

3874 + // We construct an errorInfo from the boundary's componentStack so the error in dev will indicate which

3875 + // boundary the message is referring to

3876 + const errorInfo = getThrownInfo(task.componentStack);

3877 + const trackedPostpones = request.trackedPostpones;

3878 if (boundary.status !== CLIENT_RENDERED) {

3860 - // We construct an errorInfo from the boundary's componentStack so the error in dev will indicate which

3861 - // boundary the message is referring to

3862 - const errorInfo = getThrownInfo(task.componentStack);

3879 + if (enableHalt) {

3880 + if (trackedPostpones !== null && segment !== null) {

3881 + // We are aborting a prerender

3882 + if (

3883 + enablePostpone &&

3884 + typeof error === 'object' &&

3885 + error !== null &&

3886 + error.$$typeof === REACT_POSTPONE_TYPE

3887 + ) {

3888 + const postponeInstance: Postpone = (error: any);

3889 + logPostpone(request, postponeInstance.message, errorInfo, null);

3890 + } else {

3891 + // We are aborting a prerender and must halt this boundary.

3892 + // We treat this like other postpones during prerendering

3893 + logRecoverableError(request, error, errorInfo, null);

3894 + }

3895 + trackPostpone(request, trackedPostpones, task, segment);

3896 + // If this boundary was still pending then we haven't already cancelled its fallbacks.

3897 + // We'll need to abort the fallbacks, which will also error that parent boundary.

3898 + boundary.fallbackAbortableTasks.forEach(fallbackTask =>

3899 + abortTask(fallbackTask, request, error),

3900 + );

3901 + boundary.fallbackAbortableTasks.clear();

3902 + return finishedTask(request, boundary, segment);

3903 + }

3904 + }

3905 + boundary.status = CLIENT_RENDERED;

3906 + // We are aborting a render or resume which should put boundaries

3907 + // into an explicitly client rendered state

3908 let errorDigest;

3909 if (

3910 enablePostpone &&

@@ -4145,6 +4190,43 @@ function retryRenderTask(

4190 ? request.fatalError

4191 : thrownValue;

4192

4193 + if (

4194 + enableHalt &&

4195 + request.status === ABORTING &&

4196 + request.trackedPostpones !== null

4197 + ) {

4198 + // We are aborting a prerender and need to halt this task.

4199 + const trackedPostpones = request.trackedPostpones;

4200 + const thrownInfo = getThrownInfo(task.componentStack);

4201 + task.abortSet.delete(task);

4202 +

4203 + if (

4204 + enablePostpone &&

4205 + typeof x === 'object' &&

4206 + x !== null &&

4207 + x.$$typeof === REACT_POSTPONE_TYPE

4208 + ) {

4209 + const postponeInstance: Postpone = (x: any);

4210 + logPostpone(

4211 + request,

4212 + postponeInstance.message,

4213 + thrownInfo,

4214 + __DEV__ && enableOwnerStacks ? task.debugTask : null,

4215 + );

4216 + } else {

4217 + logRecoverableError(

4218 + request,

4219 + x,

4220 + thrownInfo,

4221 + __DEV__ && enableOwnerStacks ? task.debugTask : null,

4222 + );

4223 + }

4224 +

4225 + trackPostpone(request, trackedPostpones, task, segment);

4226 + finishedTask(request, task.blockedBoundary, segment);

4227 + return;

4228 + }

4229 +

4230 if (typeof x === 'object' && x !== null) {

4231 // $FlowFixMe[method-unbinding]

4232 if (typeof x.then === 'function') {