Update web_core with Signal improvements and error handling#875
Update web_core with Signal improvements and error handling#875gspencergoog merged 4 commits intogoogle:mainfrom
web_core with Signal improvements and error handling#875Conversation
web_core with Signal improvements and error handling
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to the reactive capabilities of web_core, particularly around Signal-based state management and error handling in DataContext. The introduction of the isSignal utility for robust type checking and the new error handling within reactive effects are great steps forward.
I've identified one area for improvement in the new error handling logic: there's a case where non-A2uiExpressionError errors in reactive evaluations could be silently swallowed, making debugging difficult. I've suggested a change to ensure all errors are reported.
Overall, these are solid changes that enhance the stability and predictability of the reactive system. Addressing the feedback will further improve the robustness of the error handling.
| } catch (e: any) { | ||
| if (e instanceof A2uiExpressionError) { | ||
| this.surface.dispatchError({ | ||
| code: "EXPRESSION_ERROR", | ||
| message: e.message, | ||
| expression: e.expression, | ||
| details: e.details, | ||
| }); | ||
| } | ||
| // In reactive mode, we should not throw. Instead, reset the signal value. | ||
| resultSig.value = undefined; | ||
| } |
There was a problem hiding this comment.
The current implementation of the catch block only handles A2uiExpressionError. Any other type of error thrown during the reactive evaluation will be silently swallowed. This can make debugging very difficult as there will be no indication of what went wrong, other than the signal's value becoming undefined.
It would be more robust to handle other error types as well, for example by dispatching a generic error to the surface.
} catch (e: any) {
if (e instanceof A2uiExpressionError) {
this.surface.dispatchError({
code: "EXPRESSION_ERROR",
message: e.message,
expression: e.expression,
details: e.details,
});
} else {
this.surface.dispatchError({
code: "EXPRESSION_ERROR",
message: e.message ?? `An unexpected error occurred in function ${call.call}.`,
expression: call.call,
details: { stack: e.stack },
});
}
// In reactive mode, we should not throw. Instead, reset the signal value.
resultSig.value = undefined;
}Extracted redundant try-catch handling blocks into a dedicated `dispatchExpressionError` private helper to reduce code duplication in `evaluateFunctionReactive` and `resolveSignal` scopes. Updated `resolveDynamicValue` to utilize `evaluateFunctionReactive` directly for its function invoker path setup to remove separate fall-through flows. Added and adapted corresponding tests to assert correctly against generic error dispatches instead of original literal fallbacks.
Description
This PR updates
renderers/web_corewith Signal-based improvements to theDataContextand core models to support reactive state updates.Changes
renderers/web_core/src/rendering/data-context.ts: RefactoredDataContextto better leverage Signal patterns.renderers/web_core/src/state/*.ts: Updated state models and subscription logic to prevent reactive leaks and align with signals.renderers/web_core/src/errors.ts: Unified action/error message processor handling.Impact & Risks
Testing
renderers/web_core.