You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
factor out the stack parsing nonsense into a reusable library #4021
If we do any more stack parsing, it's time to bite the bullet and just write a library to parse them. It's becoming far too brittle to do it with handcrafted RegExps everywhere.
Can it be extracted from package:stack_trace? In fact, I'd rather keep using package:stack_trace if it provided a way to customize how the stack frames are formatted.
@nex3, is any of the above possible today or in the future?
Stack frame parsing is pretty trivial (https://github.com/dart-lang/stack_trace/blob/master/lib/src/frame.dart#L130 is the bulk of it in stack_frame). The problem with reusing stack_frame is we're paying the cost for a lot of functionality we don't need in Flutter (for example, parsing Safari stack frames). In principle the async chain support would be good too, but in practice it's not because what you really want is the conceptual stack, which you can't really derive from the chains (the chains generally just consist of a bunch of privates in dart:async, followed by a single frame or so that's the continuation of the previous code... which doesn't really match what the code looks like any more than the regular stack trace without the chaining).
My understanding was also that the VM planned to make stack-trace handling better/prettier (like handling of async by default). @a-siva or @johnmccutchan would know.
I'm not sure what you mean by "extract". The stack_trace package is already the place to which we extract stack parsing.
The problem with reusing stack_frame is we're paying the cost for a lot of functionality we don't need in Flutter (for example, parsing Safari stack frames).
Pretty much any package you use is going to have some stuff you don't use. That's the nature of packaging related functionality for re-use. Luckily Dart tools tend to have pretty good support for tree-shaking to ditch unused code before it's distributed to the public.
(the chains generally just consist of a bunch of privates in dart:async, followed by a single frame or so that's the continuation of the previous code... which doesn't really match what the code looks like any more than the regular stack trace without the chaining).
I'm surprised that you don't find stack chains useful. That's contrary to the vast majority of user feedback I get.
Terse and folded stacks usually eliminate most of the core library cruft. Even if they didn't, though, surely having full information about where an error happened is more useful than having no information, even if you have to dig through some irrelevant lines.
Nothing, I didn't realize it was public already :)
But, if we don't want to depend on Safari, then it could be extracted into a standalone package that only knows how to parse VM stack traces and not JS stack traces.
I'm surprised that you don't find stack chains useful. That's contrary to the vast majority of user feedback I get.
👍
Losing asynchronous stacks makes it much harder to debug async code. Perhaps we're not writing a lot of async code in Flutter, but I'm sure our users will. Any time you talk to the network, file system, or a Redux-like framework, you are dealing with asynchrony that's super hard to debug without chaining.
I'd be curious to see an example of how a chain is useful. I see them all the time (e.g. from flutter_test) but have literally never been able to use them to help me debug the problem.
Sure. In this commit I added a chain because when a devicelab test failed to run a process (btw, here's another async use-case right there), all we got was:
Ah, yeah, that's a good example where it's useful. Not as good as a true async view of the stack (one that only had exec. in the stack once, that is), but still, more useful than the original.
But, if we don't want to depend on Safari, then it could be extracted into a standalone package that only knows how to parse VM stack traces and not JS stack traces.
I think the current factoring of the package is fine. It doesn't depend on Safari, it just has some logic for parsing stack frames that could come from Safari. It's no different than using the collection package without calling the compareNatural() function, for example.
Not as good as a true async view of the stack (one that only had exec. in the stack once, that is), but still, more useful than the original.
The extra exec frames are actually a factor of how the VM implements async/await, not something inherent to stack chains.
I'm not sure what to do with this bug? It seems like package:stacktrace is the library? Is there a concrete bug we can file with package:stacktrace, or does this bug still serve utility?
Now that we're teaching crash reporting back-ends about Dart stack traces it would help to keep the number of different formats to a minimum. This also means that future changes to stack trace formats (VM or package:stack_trace) should be planned and driven by some level of consensus across teams.
We would like to avoid adding new dependencies (and remove many of our existing ones), for reasons we can discuss offline, but that is one reason not to just defer to another package (especially for something this trivial). If we do this, we should just implement it in the foundation layer. I would have done this already except that the stack logic on the Dart side has been in flux (e.g. adding an annotation for asynchronous continuations and so on) and may change some more (e.g. I've been hoping the StackTrace class itself would become more introspectable), so I've been waiting for that to settle down.
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.
Activity
yjbanov commentedon Nov 14, 2016
Can it be extracted from
package:stack_trace? In fact, I'd rather keep usingpackage:stack_traceif it provided a way to customize how the stack frames are formatted.@nex3, is any of the above possible today or in the future?
Hixie commentedon Nov 14, 2016
Stack frame parsing is pretty trivial (https://github.com/dart-lang/stack_trace/blob/master/lib/src/frame.dart#L130 is the bulk of it in stack_frame). The problem with reusing stack_frame is we're paying the cost for a lot of functionality we don't need in Flutter (for example, parsing Safari stack frames). In principle the async chain support would be good too, but in practice it's not because what you really want is the conceptual stack, which you can't really derive from the chains (the chains generally just consist of a bunch of privates in dart:async, followed by a single frame or so that's the continuation of the previous code... which doesn't really match what the code looks like any more than the regular stack trace without the chaining).
eseidelGoogle commentedon Nov 14, 2016
My understanding was also that the VM planned to make stack-trace handling better/prettier (like handling of async by default). @a-siva or @johnmccutchan would know.
nex3 commentedon Nov 14, 2016
I'm not sure what you mean by "extract". The
stack_tracepackage is already the place to which we extract stack parsing.Pretty much any package you use is going to have some stuff you don't use. That's the nature of packaging related functionality for re-use. Luckily Dart tools tend to have pretty good support for tree-shaking to ditch unused code before it's distributed to the public.
I'm surprised that you don't find stack chains useful. That's contrary to the vast majority of user feedback I get.
Terse and folded stacks usually eliminate most of the core library cruft. Even if they didn't, though, surely having full information about where an error happened is more useful than having no information, even if you have to dig through some irrelevant lines.
yjbanov commentedon Nov 14, 2016
Nothing, I didn't realize it was public already :)
But, if we don't want to depend on Safari, then it could be extracted into a standalone package that only knows how to parse VM stack traces and not JS stack traces.
👍
Losing asynchronous stacks makes it much harder to debug async code. Perhaps we're not writing a lot of async code in Flutter, but I'm sure our users will. Any time you talk to the network, file system, or a Redux-like framework, you are dealing with asynchrony that's super hard to debug without chaining.
Hixie commentedon Nov 14, 2016
I'd be curious to see an example of how a chain is useful. I see them all the time (e.g. from flutter_test) but have literally never been able to use them to help me debug the problem.
yjbanov commentedon Nov 14, 2016
Sure. In this commit I added a chain because when a devicelab test failed to run a process (btw, here's another async use-case right there), all we got was:
After the change it reports this:
A-ha! So a
StartUptest is trying to run afluttercommand which fails. I can now go to that code and try to debug it.Hixie commentedon Nov 14, 2016
Ah, yeah, that's a good example where it's useful. Not as good as a true async view of the stack (one that only had exec. in the stack once, that is), but still, more useful than the original.
nex3 commentedon Nov 15, 2016
I think the current factoring of the package is fine. It doesn't depend on Safari, it just has some logic for parsing stack frames that could come from Safari. It's no different than using the
collectionpackage without calling thecompareNatural()function, for example.The extra
execframes are actually a factor of how the VM implementsasync/await, not something inherent to stack chains.eseidelGoogle commentedon Feb 23, 2017
I'm not sure what to do with this bug? It seems like package:stacktrace is the library? Is there a concrete bug we can file with package:stacktrace, or does this bug still serve utility?
yjbanov commentedon Feb 23, 2017
Now that we're teaching crash reporting back-ends about Dart stack traces it would help to keep the number of different formats to a minimum. This also means that future changes to stack trace formats (VM or
package:stack_trace) should be planned and driven by some level of consensus across teams.Hixie commentedon Feb 24, 2017
We would like to avoid adding new dependencies (and remove many of our existing ones), for reasons we can discuss offline, but that is one reason not to just defer to another package (especially for something this trivial). If we do this, we should just implement it in the foundation layer. I would have done this already except that the stack logic on the Dart side has been in flux (e.g. adding an annotation for asynchronous continuations and so on) and may change some more (e.g. I've been hoping the StackTrace class itself would become more introspectable), so I've been waiting for that to settle down.
Hixie commentedon Mar 3, 2017
One of the things this would enable is fixing the "ourFault" logic in assertions.dart in a more robust way.
github-actions commentedon Aug 17, 2021
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.