[compiler] Remove tryRecord, add catch-all error handling, fix remaining throws

[everettbu]

Mirror of facebook/react#35837
Original author: josephsavona

---

Remove tryRecord() from the compilation pipeline now that all passes record
errors directly via env.recordError() / env.recordErrors(). A single
catch-all try/catch in Program.ts provides the safety net for any pass that
incorrectly throws instead of recording.

Key changes:

• Remove all ~64 env.tryRecord() wrappers in Pipeline.ts
• Delete tryRecord() method from Environment.ts
• Add CompileUnexpectedThrow logger event so thrown errors are detectable
• Log CompileUnexpectedThrow in Program.ts catch-all for non-invariant throws
• Fail snap tests on CompileUnexpectedThrow to surface pass bugs in dev
• Convert throwTodo/throwDiagnostic calls in HIRBuilder (fbt, this),
  CodegenReactiveFunction (for-in/for-of), and BuildReactiveFunction to
  record errors or use invariants as appropriate
• Remove try/catch from BuildHIR's lower() since inner throws are now recorded
• CollectOptionalChainDependencies: return null instead of throwing on
  unsupported optional chain patterns (graceful optimization skip)

---

Stack created with Sapling. Best reviewed with ReviewStack.

• #35838
• -> #35837
• #35836
• #35835
• #35834
• #35833
• #35832
• #35831
• #35830
• #35829
• #35828

[greptile-apps]

Greptile Summary

This PR completes the fault tolerance refactor for the React Compiler by removing all ~64 env.tryRecord() wrappers from Pipeline.ts and replacing them with direct error recording via env.recordError() / env.recordErrors() on the Environment. The compilation pipeline now accumulates errors instead of stopping at the first one, with a final env.hasErrors() check at the end of runWithEnvironment() to return aggregated errors.

• Error accumulation: Environment.ts gains #errors, recordError(), recordErrors(), hasErrors(), and aggregateErrors() methods. Invariant-category errors are still immediately thrown since they represent unrecoverable internal bugs.
• Pipeline simplification: Pipeline.ts removes all .unwrap() calls from validation/pass results. Functions like lower(), codegenFunction(), and all validators now return void or direct values instead of Result types, recording errors on the environment.
• Catch-all safety net: Program.ts adds CompileUnexpectedThrow logger event detection in catch blocks for both tryCompileFunction and retryCompileFunction, so passes that incorrectly throw instead of recording are detectable in development.
• Graceful degradation in BuildHIR: Adds best-effort handling for for(;;) (empty init/test), var declarations (treated as let), and unsupported for-loop inits. Instead of aborting on these patterns, errors are recorded and compilation continues with reasonable fallbacks.
• Test infrastructure: Snap tests now fail on CompileUnexpectedThrow events. New fault tolerance test fixtures validate that multiple independent errors are reported together.
• ~70 test fixture updates: Expected outputs updated to reflect multi-error reporting and changed error formatting.

Confidence Score: 4/5

• This PR is a well-structured refactor with comprehensive test coverage; safe to merge with only minor style suggestions.
• Score reflects a large but systematic refactor that consistently applies one pattern (error recording instead of throwing) across ~20 source files. The changes are well-tested with new fault tolerance fixtures and updated snapshot expectations. The only issues found are minor style inconsistencies (unconditional recordErrors calls, one as any cast). No logical bugs or regressions identified.
• compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts has the most complex behavioral changes (graceful for-loop/var handling) and deserves careful review of the fallback logic.

Important Files Changed

Filename	Overview
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts	Core pipeline changes: removes all ~15 .unwrap() calls from validation/pass results, changes runWithEnvironment to return Result<CodegenFunction, CompilerError>, and adds final env.hasErrors() check at pipeline end. Clean, systematic refactor.
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts	Adds CompileUnexpectedThrow logging in catch-all handlers for both tryCompileFunction and retryCompileFunction. Uses Result pattern to distinguish expected errors from unexpected throws. Correct handling of invariant vs non-invariant errors.
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts	Adds #errors field and recordError/recordErrors/hasErrors/aggregateErrors methods. The recordError method correctly rethrows Invariant-category errors immediately since they represent unrecoverable internal bugs.
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts	Significant changes: lower() now returns HIRFunction directly instead of Result, records errors on env after builder.build(). Adds graceful handling for for(;;) (empty init/test), var declarations (treated as let), and removes early returns on nested function lowering failures. The effects field is correctly removed from the return object to match the updated HIRFunction type.
compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts	Converts throwDiagnostic/throwTodo calls for fbt, this, and unreachable hoisted declarations to builder.errors.push() instead of throwing, enabling error accumulation.
compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts	Returns null instead of throwing on unsupported optional chain patterns, gracefully skipping the optimization.
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts	Converts throwTodo to error recording with early return t.emptyStatement() for for-in/for-of StoreContext and non-trivial init cases. Changes codegenFunction/codegenReactiveFunction return types from Result to direct values, recording errors on fn.env.
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts	Converts throwTodo for unexpected terminal kind to CompilerError.invariant(false, ...), which is appropriate since this represents an internal logic error that should not be silently swallowed.
compiler/packages/snap/src/compiler.ts	Adds snap test check for CompileUnexpectedThrow events, failing tests when passes throw instead of recording. Uses (l.event as any).data type assertion.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts	Refactored to pass errors: CompilerError through the recursive implementation. Uses innerErrors for function expressions to determine ref access effects without polluting the outer error list — consistent with previous behavior.
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts	Changes return type from Result to direct array. Adds fn.env.enableValidations guard to match the previous Pipeline.ts caller behavior.

_{Last reviewed commit: 6163309}

[greptile-apps]

_{98 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Unsafe as any type cast
The (l.event as any).data cast bypasses type safety when the type could be narrowed from the filter. Consider using a type guard or asserting the narrowed event type directly:

Suggested change

	unexpectedThrows.map(l => (l.event as any).data).join('\n'),
	unexpectedThrows.map(l => (l.event as CompileUnexpectedThrowEvent).data).join('\n'),

Alternatively, a type predicate on the filter would avoid the cast entirely.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: compiler/packages/snap/src/compiler.ts
Line: 389

Comment:
**Unsafe `as any` type cast**
The `(l.event as any).data` cast bypasses type safety when the type could be narrowed from the filter. Consider using a type guard or asserting the narrowed event type directly:

```suggestion
        unexpectedThrows.map(l => (l.event as CompileUnexpectedThrowEvent).data).join('\n'),
```

Alternatively, a type predicate on the filter would avoid the cast entirely.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Unconditional recordErrors call
This calls fn.env.recordErrors(errors) without checking errors.hasAnyErrors() first. While harmless (empty details array is a no-op), it's inconsistent with the pattern used in most other validators (e.g., ValidateHooksUsage, ValidateUseMemo, ValidateNoCapitalizedCalls). The same inconsistency also appears in ValidatePreservedManualMemoization, ValidateNoDerivedComputationsInEffects, and ValidateSourceLocations.

Suggested change

	fn.env.recordErrors(errors);
	if (errors.hasAnyErrors()) {
	fn.env.recordErrors(errors);
	}

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateMemoizedEffectDependencies.ts
Line: 54

Comment:
**Unconditional `recordErrors` call**
This calls `fn.env.recordErrors(errors)` without checking `errors.hasAnyErrors()` first. While harmless (empty details array is a no-op), it's inconsistent with the pattern used in most other validators (e.g., `ValidateHooksUsage`, `ValidateUseMemo`, `ValidateNoCapitalizedCalls`). The same inconsistency also appears in `ValidatePreservedManualMemoization`, `ValidateNoDerivedComputationsInEffects`, and `ValidateSourceLocations`.

```suggestion
  if (errors.hasAnyErrors()) {
    fn.env.recordErrors(errors);
  }
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[everettbu]

Upstream PR was closed or merged. Code is synced via branch mirror.