Fix remaining holes in stack trace demangling#60478
Conversation
fluttergithubbot
added
a: tests
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
Only real concern is removing the test - it's not entirely clear to me why we should allow that test to fail now.
|
I actually renamed the file then boiled it down to what it is now, which is basically testing if FlutterErrorDetails.toDiagnosticsNode fails. Looking back at it, the added redundancy of a slightly more practical use case is probably a good thing so I'll just re-introduce the original test. |
|
Weird, your async_gap_test.dart doesn't fail if I comment out the assignment of demangleStackTrace, where was it supposed to fail before? |
|
I think as long as the test isn't failing it's fine, but we should keep the test. If something's broken, the test should end up failing on your new assert that we don't have a line like |
* Implement stack trace demangling in the framework.
flutter#60916) This reverts commit d986fdc.
…r#60478)" (flutter#60916)" (flutter#60996) This reverts commit 2c7e5dd.
4 participants
Description
Follow-up of #59900
Instead of forcing every implementer of
FlutterError.onError(both internally and externally) to manually demangle stack traces, we cover the small number of places in the framework that parse them.StackFrameparsing when it encounters package:stack_trace async gap delimiters, this prevents people from assuming the vm's own stack trace format changed, as we all initially assumed.FlutterError.demangleStackTraceto allow testing environments to provide their own demangle callbacks.FlutterError.defaultStackFilterorStackFrame.fromStackStringcould be passed an external stack trace.Related Issues
Fixes any remaining instances of #59893
Breaking Change
None of these changes should be breaking, and all local tests are passing.