web
web copied to clipboard
Fix incorrect non-null assertion
I believe _target can be null here.
mmm - I'm inclined to tighten this logic differently.
I believe the intended invariant was to have 2 states for the event stream subscriptions:
- active:
_onDataand_targetare not null - inactive:
_onDatais null. Typically subscriptions are inactive either because theonDatahandler hasn't been set or because the subscription was cancelled.
One way to tighten this differently would be to make the constructor parameter stricter and only accept a non-null _target value. Another option is to keep the type signature as it is, but immediately cancel the subscription when the provided _target is null (since there is no API to set the target after the subscription was created).
For reference - this non-null assertion has been in this logic not only here, but also in the equivalent logic we had in dart:html where this was ported from.
I'm curious - what scenario made you run into this issue, @mnordine?
One way to tighten this differently would be to make the constructor parameter stricter and only accept a non-null _target value.
I agree, just not sure why it wasn't done like that in the first place.
I'm curious - what scenario made you run into this issue, @mnordine?
I was looking into a stack trace generated from Sentry for our Dart web app that uses web:
NoSuchMethodError: Null check operator used on a null value
at A._EventStreamSubscription.cancel.prototypeLc(package:web/src/helpers/events/streams.dart:258:9)
at A.aS(package:web/src/helpers/events/streams.dart:160:3)
at App.start(package:web/src/helpers/events/streams.dart:130:7)
at A.bsd(org-dartlang-sdk:///lib/_internal/js_runtime/lib/async_patch.dart:303:19)
at dartProgram(main.c20686344c6ca935.js:329:4)
at dartProgram(main.c20686344c6ca935.js:279:67)
at A._rootRunUnary(org-dartlang-sdk:///lib/async/zone.dart:1419:1)
at installTearOffs(main.c20686344c6ca935.js:1405:3)
at A.Gi.prototype_CustomZone.runUnary(org-dartlang-sdk:///lib/async/zone.dart:1315:12)
at A._Future._propagateToListeners.handleValueCallback._Future._propagateToListeners.handleValueCallback$0(org-dartlang-sdk:///lib/async/future_impl.dart:862:13)
So I started looking into it.
I want to stress that the stacks generated from sentry-cli are very poor, and we have never been able to figure it out. It seems Sentry isn't terribly interested in fixing stack traces for the web.
Do you by chance have access to the original js stack trace as well as as the source-map of the compiled code at the time this stack was produced? We have a script dart2js_tools/bin/deobfuscate that does similar work to what sentry does, but that includes logic for some extensions added by dart2js (including lookup of minified names and expansion of inlined stack frames). I have doubts that it will help in this example, but I'm curious how they compare.
Sorry, I don't have it. Will update here if I'm able to get it.
Going to close out this PR. Please reopen when you want to dig in again!