Repository navigation
fix(mix): report FlutterErrorDetails in debug mode when Prop value conversion fails - #1096
leoafarias wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9c8f1ad49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| debugPrint( | ||
| 'Mix: could not convert value of type ${value.runtimeType} ' | ||
| 'to Mix<$V>. Register a MixConverter for <$V> or pass a Mix via Prop.mix.', | ||
| FlutterError.reportError( |
There was a problem hiding this comment.
Throw instead of only reporting conversion failures
When a converter is missing in a debug build, FlutterError.reportError invokes the global FlutterError.onError handler and then returns; it does not throw. Consequently, resolveProp still drops the unconverted value and returns the remaining Mix result, and a handler can swallow the error exactly as this new test does, so the intended fail-fast behavior is not implemented. Throw the FlutterError from the assertion block and test with throwsA if resolution is meant to abort.
Useful? React with 👍 / 👎.
Summary
Replaces the silent
debugPrint('Mix: could not convert value of type ...')insideassertinProp.resolvePropwithFlutterError.reportError(FlutterErrorDetails(...))so that failedMix<V>conversions surface through Flutter's standard error pipeline (FlutterError.onError) in debug/test mode while still returning the fallback resolved value without crashing the widget build.Root Cause & Motivation
When a
Prop<V>accumulates both aMix<V>source (e.g. a customMix<double>subclass passed viaProp.mix(...)) and a plain value source (ValueSource<V>orTokenSource<V>resolving toV),hasMixValuesistrueandProp.resolvePropconverts every non-Mix<V>entry viaMixConverterRegistry.instance.tryConvert<V>(value).Previously, when no
MixConverter<V>was registered (such as for scalardouble),Prop.resolveProponly calleddebugPrintinside anassertblock and silently dropped the unconverted value. BecausedebugPrintdoes not route throughFlutterError.onError, invalidMix<V>+ scalarVcombinations went unnoticed influtter testwhile printing console noise on every animation frame.Using
FlutterError.reportError(FlutterErrorDetails(...))inside theassert(() { ... return true; }())block follows Flutter's standard non-fatal error reporting pattern (likeRenderFlexoverflow):resolvePropcontinues and returns the resolved fallback value in both debug and release builds instead of throwing mid-build.assertblock is stripped in release builds.flutter testautomatically:TestWidgetsFlutterBindinghooksFlutterError.onError, so widget tests fail immediately (andtester.takeException()captures theFlutterError).Verification
packages/mix/test/src/core/prop_test.dartverifying that merging aValueSource<double>with a customMix<double>reportsFlutterErrorDetailsviaFlutterError.onErrorand still returns the resolved fallback value (20.0).flutter test test/src/core/prop_test.dartdart analyze lib/src/core/prop.dart test/src/core/prop_test.dart