fix(bloc): omit synthetic BlocBase.addError frame from auto-captured stack traces#4808
fix(bloc): omit synthetic BlocBase.addError frame from auto-captured stack traces#4808realmeylisdev wants to merge 3 commits into
BlocBase.addError frame from auto-captured stack traces#4808Conversation
|
@felangel — would appreciate a review when you have time. One open question flagged in the description: should this go out as 9.3.0 (minor, since the |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Thanks for the PR! I'm not sure if this is the correct fix though 🤔 |
|
I tested this on https://dartpad.dev/ void main() {
print('hello');
print(Test.callThis(true));
}
class Test {
static StackTrace callThis(bool withoutTopFrame) {
return thenCallThis(withoutTopFrame);
}
static StackTrace thenCallThis(bool withoutTopFrame) {
if (withoutTopFrame) {
return _withoutTopFrame(StackTrace.current);
}
return StackTrace.current;
}
static StackTrace _withoutTopFrame(StackTrace trace) {
final raw = trace.toString();
final newline = raw.indexOf('\n');
if (newline < 0) return trace;
return StackTrace.fromString(raw.substring(newline + 1));
}
}
I was expecting |
Status
READY
Breaking Changes
NO
Description
Closes #3585.
When
BlocBase.addError(error)is called without an explicitStackTrace, the trace auto-captured viaStackTrace.currentalways hasBlocBase.addErroritself as its top frame. This obscures the caller's actual call site in error reporters such as Crashlytics/Sentry, and forces every project to ship a customBlocObserverthat strips the frame back out (the workaround documented in the issue).This PR strips the synthetic top frame from the auto-captured trace at the source, so the resulting
StackTracebegins at the caller's call site.Reproducing the original bug (before this fix)
Before: prints
#0 BlocBase.addError (package:bloc/src/bloc_base.dart:145:33)— the bloc framework hides the actual call site.After: prints
#0 MyCubit.doWork (file:///path/to/my_cubit.dart:5:5)— the user's call site is the top frame.Implementation notes
StackTrace.currentis captured inline insideaddError(not via a helper), which pins the synthetic-frame depth at exactly 1 across VM, AOT, and dart2js, regardless of inlining decisions.BlocBase._withoutTopFrameperforms string-only manipulation (StackTrace.fromStringafter dropping the first textual line) — it doesn't capture a trace itself, so it doesn't affect frame depth.member == 'BlocBase.addError') breaks. @maRci002 raised exactly this concern in the issue thread.newline < 0fallback returns the trace unchanged for the single-frame edge case (defensive — covers any unexpected platform behavior such as web traces collapsing to one line).Why a minor (9.3.0) version bump
The
StackTraceflowing intoBlocObserver.onErroris part of the observable public surface; tooling that buckets/asserts on trace shape will see a one-frame-shorter trace. SemVer-strict reading argues for minor. Happy to drop to 9.2.1 (patch) if you'd rather treat this as a pure bug fix — flagging this for your call.Type of Change
Test Plan
Three tests added to the existing
group('addError', …)inpackages/bloc/test/cubit_test.dart:omits BlocBase.addError frame when no stackTrace is passed— callsaddError(error)and asserts the captured trace's first line contains neitherBlocBase.addErrornorbloc_base.dart.passes through an explicit stackTrace unchanged— regression guard for the explicit-trace path; assertssame(explicit).captured stackTrace is non-empty when no stackTrace is passed— sanity guard against over-trimming.Verification:
dart testfrompackages/bloc/— 119/119 passing (all existing + 3 new).dart analyzefrompackages/bloc/— No issues found.dart test -p chrome) couldn't be executed locally (no Chrome installed in dev environment) and CI doesn't currently exercise browser tests forpkg:bloc. Thenewline < 0fallback keeps web safe-by-default — worst case we don't trim, we don't regress.Notes
package:blocdependent onmetaonly).flutter_bloc,bloc_test,hydrated_bloc,replay_bloc, orEmitter.forEach/Emitter.onEach. Verified that none of those paths route errors throughBlocBase.addError's auto-capture branch — they either usecompleter.completeErroror pass user-supplied stack traces through.