fix(hydration): route mismatches via handleError when consumer set#14757
fix(hydration): route mismatches via handleError when consumer set#14757pierluigilenoci wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughHydration mismatch reporting is routed through Vue's error pipeline: add ErrorCodes.HYDRATION_MISMATCH, implement logMismatchError(instance) with a dedupe guard and resetHydrationMismatchState(), update mismatch call sites to pass component context, and add tests verifying handlers receive the error. ChangesHydration mismatch handling
Sequence DiagramsequenceDiagram
participant Hydration as HydrationSystem
participant Log as logMismatchError
participant Handler as handleError
participant App as app.config.errorHandler
participant Comp as onErrorCaptured
Hydration->>Log: detect mismatch (parentComponent)
Log->>Log: check hasLoggedMismatchError
alt not logged
Log->>Log: set hasLoggedMismatchError = true
Log->>Handler: handleError(Error("Hydration completed but contains mismatches."), instance, HYDRATION_MISMATCH)
Handler->>App: invoke if configured
Handler->>Comp: invoke if ancestor captured error
else already logged
Log->>Log: skip duplicate emission
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/runtime-core/src/hydration.ts`:
- Around line 68-92: The current call to handleError(...) always routes through
logError() and emits extra dev warnings; change the logic so handleError is only
invoked when there is an actual consumer (check
instance.appContext.config.errorHandler or hasErrorCaptured(instance)), and when
no consumer exists fall back to directly calling console.error(new
Error('Hydration completed but contains mismatches.')) instead of handleError;
update both the __TEST__ branch and the default branch to use this gating around
handleError and preserve the existing ErrorCodes.HYDRATION_MISMATCH constant
when routing to handleError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4511270-f4c7-4adc-aa7e-d07fa84ea855
📒 Files selected for processing (3)
packages/runtime-core/__tests__/hydration.spec.tspackages/runtime-core/src/errorHandling.tspackages/runtime-core/src/hydration.ts
| // Route through Vue's error handling pipeline so that | ||
| // onErrorCaptured and app.config.errorHandler can catch it. | ||
| if (__TEST__) { | ||
| // In test mode, only route through handleError if an error handler is | ||
| // actually configured (errorHandler or onErrorCaptured), to avoid adding | ||
| // unexpected "Unhandled error" warnings to every mismatch test. | ||
| if ( | ||
| instance && | ||
| (instance.appContext.config.errorHandler || hasErrorCaptured(instance)) | ||
| ) { | ||
| handleError( | ||
| new Error('Hydration completed but contains mismatches.'), | ||
| instance, | ||
| ErrorCodes.HYDRATION_MISMATCH, | ||
| false, | ||
| ) | ||
| } | ||
| } else { | ||
| handleError( | ||
| new Error('Hydration completed but contains mismatches.'), | ||
| instance, | ||
| ErrorCodes.HYDRATION_MISMATCH, | ||
| false, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Preserve the old fallback when no error handler is present.
handleError(..., false) still goes through logError(), which adds a new dev warning (Unhandled error during execution of hydration) before console.error(err). That means unhandled hydration mismatches now emit extra generic noise in dev, even though the old behavior was just the specific mismatch warning plus the one-off console error.
Please gate the handleError() path on an actual consumer (app.config.errorHandler / ancestor errorCaptured) and keep the old direct console.error(new Error(...)) fallback otherwise.
Proposed fix
const logMismatchError = (
instance: ComponentInternalInstance | null = null,
) => {
if (hasLoggedMismatchError) {
return
}
hasLoggedMismatchError = true
- // Route through Vue's error handling pipeline so that
- // onErrorCaptured and app.config.errorHandler can catch it.
- if (__TEST__) {
- // In test mode, only route through handleError if an error handler is
- // actually configured (errorHandler or onErrorCaptured), to avoid adding
- // unexpected "Unhandled error" warnings to every mismatch test.
- if (
- instance &&
- (instance.appContext.config.errorHandler || hasErrorCaptured(instance))
- ) {
- handleError(
- new Error('Hydration completed but contains mismatches.'),
- instance,
- ErrorCodes.HYDRATION_MISMATCH,
- false,
- )
- }
- } else {
- handleError(
- new Error('Hydration completed but contains mismatches.'),
- instance,
- ErrorCodes.HYDRATION_MISMATCH,
- false,
- )
- }
+ const err = new Error('Hydration completed but contains mismatches.')
+ const hasConsumer =
+ !!instance &&
+ (instance.appContext.config.errorHandler || hasErrorCaptured(instance))
+
+ if (hasConsumer) {
+ handleError(err, instance, ErrorCodes.HYDRATION_MISMATCH, false)
+ } else if (!__TEST__) {
+ console.error(err)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Route through Vue's error handling pipeline so that | |
| // onErrorCaptured and app.config.errorHandler can catch it. | |
| if (__TEST__) { | |
| // In test mode, only route through handleError if an error handler is | |
| // actually configured (errorHandler or onErrorCaptured), to avoid adding | |
| // unexpected "Unhandled error" warnings to every mismatch test. | |
| if ( | |
| instance && | |
| (instance.appContext.config.errorHandler || hasErrorCaptured(instance)) | |
| ) { | |
| handleError( | |
| new Error('Hydration completed but contains mismatches.'), | |
| instance, | |
| ErrorCodes.HYDRATION_MISMATCH, | |
| false, | |
| ) | |
| } | |
| } else { | |
| handleError( | |
| new Error('Hydration completed but contains mismatches.'), | |
| instance, | |
| ErrorCodes.HYDRATION_MISMATCH, | |
| false, | |
| ) | |
| } | |
| const logMismatchError = ( | |
| instance: ComponentInternalInstance | null = null, | |
| ) => { | |
| if (hasLoggedMismatchError) { | |
| return | |
| } | |
| hasLoggedMismatchError = true | |
| const err = new Error('Hydration completed but contains mismatches.') | |
| const hasConsumer = | |
| !!instance && | |
| (instance.appContext.config.errorHandler || hasErrorCaptured(instance)) | |
| if (hasConsumer) { | |
| handleError(err, instance, ErrorCodes.HYDRATION_MISMATCH, false) | |
| } else if (!__TEST__) { | |
| console.error(err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/runtime-core/src/hydration.ts` around lines 68 - 92, The current
call to handleError(...) always routes through logError() and emits extra dev
warnings; change the logic so handleError is only invoked when there is an
actual consumer (check instance.appContext.config.errorHandler or
hasErrorCaptured(instance)), and when no consumer exists fall back to directly
calling console.error(new Error('Hydration completed but contains mismatches.'))
instead of handleError; update both the __TEST__ branch and the default branch
to use this gating around handleError and preserve the existing
ErrorCodes.HYDRATION_MISMATCH constant when routing to handleError.
|
Hi — friendly ping. Is this PR still on the radar for review? Happy to rebase or make changes if needed. Thanks! |
|
I agree the observability use case is valid, but I don't think this should be merged in the current form. Routing hydration mismatch reporting through I think the safer direction is to preserve the current default behavior and only expose hydration mismatch reporting to user handlers when there is an explicit consumer, or consider an app-level / warning-handler based API instead of putting it fully into the component error boundary pipeline. |
|
Hi — friendly follow-up. CI is green and all checks pass. Would you be able to review when you get a chance? Thank you! |
pierluigilenoci
force-pushed
the
fix/hydration-error-handler
branch
from
5e1c9ff to
81d3a55
Compare
|
@edison1105 thanks for the careful review — fully agree. I just pushed a rework that addresses your concerns:
Without a consumer, the per-mismatch Other concerns:
Tests cover all four cases: Let me know if the gating-on-consumer shape works for you, or if you'd prefer the app-level / warning-handler API instead — happy to iterate. |
pierluigilenoci
changed the title
pierluigilenoci
changed the title
Hydration mismatch errors were never routed through Vue's error handling pipeline, so onErrorCaptured and app.config.errorHandler could not catch them — making it hard to wire hydration mismatch reporting into observability tools (fix vuejs#13154). logMismatchError now calls handleError(..., HYDRATION_MISMATCH, false) only when an explicit consumer is present: - app.config.errorHandler is set, or - some ancestor has registered onErrorCaptured. Without a consumer, the per-mismatch warn() calls at the call sites remain the only output, matching the prior default behavior — no 'Unhandled error during execution of hydration' warning is emitted for SSR apps that have not opted in. Adds ErrorCodes.HYDRATION_MISMATCH plus its 'hydration' info label, passes the parent component instance to logMismatchError() at every call site, and adds tests covering: app.config.errorHandler consumer, onErrorCaptured consumer, single-emit semantics across multiple mismatches, and the no-consumer path. Signed-off-by: Pierluigi Lenoci <pierluigilenoci@gmail.com>
pierluigilenoci
force-pushed
the
fix/hydration-error-handler
branch
from
81d3a55 to
68656d1
Compare
2 participants
Summary
Hydration mismatch errors were never routed through Vue's error handling pipeline, so
onErrorCapturedandapp.config.errorHandlercould not catch them. This made it impossible to wire hydration-mismatch reporting into observability tools (Sentry, Datadog, etc.) — see issue #13154.This PR routes hydration mismatches through
handleError, but only when an explicit consumer is present, so SSR apps that have not opted in see no behavior change.Behavior
logMismatchErrorcallshandleError(..., ErrorCodes.HYDRATION_MISMATCH, false)only when one of the following is true on the parent component instance:instance.appContext.config.errorHandleris set, oronErrorCaptured.Without a consumer, the per-mismatch
warn()calls at the call sites remain the only output — matching the prior default behavior. No "Unhandled error during execution of hydration" warning is emitted for SSR apps that have not opted in.Changes
packages/runtime-core/src/errorHandling.ts— addsErrorCodes.HYDRATION_MISMATCHand its'hydration'info label.packages/runtime-core/src/hydration.ts—logMismatchErrornow takes the parent component instance, gateshandleErroron consumer presence (errorHandleror any ancestor withonErrorCaptured), and is invoked withparentComponentat every mismatch call site.packages/runtime-core/__tests__/hydration.spec.ts— adds 4 tests covering:app.config.errorHandlerconsumer,onErrorCapturedconsumer, single-emit semantics across multiple mismatches, and the no-consumer path (asserts the unhandled-error warning is not emitted).Reviewer feedback addressed
@edison1105 raised concerns about semantics changes for apps that had already configured
errorHandler/onErrorCaptured, and about the unhandled-error warning fallback for default apps. The current revision keeps default behavior unchanged and only opens the integration to apps that have explicitly opted in by registering a consumer.Test plan
pnpm vitest run packages/runtime-core/__tests__/hydration.spec.ts— 108 tests passpnpm lint— cleanpnpm check(tsc) — cleanmainCloses #13154.