Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled http exceptions from the dds server / shelf_proxy #84113

Closed
jonahwilliams opened this issue Jun 7, 2021 · 12 comments · Fixed by #89591
Closed

Unhandled http exceptions from the dds server / shelf_proxy #84113

jonahwilliams opened this issue Jun 7, 2021 · 12 comments · Fixed by #89591
Assignees
Labels
c: crash Stack traces logged to the console dependency: dart Dart team may need to help us P0 Critical issues such as a build break or regression tool Affects the "flutter" command-line tool. See also t: labels. waiting for PR to land (fixed) A fix is in flight

Comments

@jonahwilliams
Copy link
Member

The number 2 crash by frequency / number 3 by affected users is an unhandled http exception from package:http.

The stack trace does not show the actual callsite, but this can be traced back to a shelf proxy usage in dds at https://github.com/dart-lang/sdk/blob/master/pkg/dds/lib/src/dds_impl.dart#L267

Thread 0 main threadClientException: Connection closed while receiving data
at IOClient.send.<anonymous closure>(io_client.dart:49)
at _invokeErrorHandler(async_error.dart:45)
at _HandleErrorStream._handleError(stream_pipe.dart:272)
at _ForwardingStreamSubscription._handleError(stream_pipe.dart:157)
at _HttpClientResponse.listen.<anonymous closure>(http_impl.dart:712)
at _rootRunBinary(zone.dart:1378)
at _CustomZone.runBinary(zone.dart:1272)
at _CustomZone.runBinaryGuarded(zone.dart:1178)
at _BufferingStreamSubscription._sendError.sendError(stream_impl.dart:360)
at _BufferingStreamSubscription._sendError(stream_impl.dart:378)
at _BufferingStreamSubscription._addError(stream_impl.dart:280)
at _ForwardingStreamSubscription._addError(stream_pipe.dart:128)
at _addErrorWithReplacement(stream_pipe.dart:176)
at _HandleErrorStream._handleError(stream_pipe.dart:277)
at _ForwardingStreamSubscription._handleError(stream_pipe.dart:157)
at _rootRunBinary(zone.dart:1378)
at _CustomZone.runBinary(zone.dart:1272)
at _CustomZone.runBinaryGuarded(zone.dart:1178)
at _BufferingStreamSubscription._sendError.sendError(stream_impl.dart:360)
at _BufferingStreamSubscription._sendError(stream_impl.dart:378)
at _BufferingStreamSubscription._addError(stream_impl.dart:280)
at _SyncStreamControllerDispatch._sendError(stream_controller.dart:737)
at _StreamController._addError(stream_controller.dart:615)
at _StreamController.addError(stream_controller.dart:569)
at _HttpParser._reportBodyError(http_parser.dart:1183)
at _HttpParser._onDone(http_parser.dart:890)
at _rootRun(zone.dart:1346)
at _CustomZone.run(zone.dart:1258)
at _CustomZone.runGuarded(zone.dart:1162)
at _BufferingStreamSubscription._sendDone.sendDone(stream_impl.dart:394)
at _BufferingStreamSubscription._sendDone(stream_impl.dart:404)
at _BufferingStreamSubscription._close(stream_impl.dart:291)
at _SyncStreamControllerDispatch._sendDone(stream_controller.dart:741)
at _StreamController._closeUnchecked(stream_controller.dart:596)
at _StreamController.close(stream_controller.dart:589)
at _Socket._onData(socket_patch.dart:2167)
at _rootRunUnary(zone.dart:1362)
at _CustomZone.runUnary(zone.dart:1265)
at _CustomZone.runUnaryGuarded(zone.dart:1170)
at _BufferingStreamSubscription._sendData(stream_impl.dart:341)
at _BufferingStreamSubscription._add(stream_impl.dart:271)
at _SyncStreamControllerDispatch._sendData(stream_controller.dart:733)
at _StreamController._add(stream_controller.dart:607)
at _StreamController.add(stream_controller.dart:554)
at new _RawSocket.<anonymous closure>(socket_patch.dart:1703)
at _NativeSocket.issueReadEvent.issue(socket_patch.dart:1201)
at _rootRun(zone.dart:1346)
at _CustomZone.run(zone.dart:1258)
at _CustomZone.runGuarded(zone.dart:1162)
at _CustomZone.bindCallbackGuarded.<anonymous closure>(zone.dart:1202)
at _rootRun(zone.dart:1354)
at _CustomZone.run(zone.dart:1258)
at _CustomZone.runGuarded(zone.dart:1162)
at _CustomZone.bindCallbackGuarded.<anonymous closure>(zone.dart:1202)
at _microtaskLoop(schedule_microtask.dart:40)
at _startMicrotaskLoop(schedule_microtask.dart:49)
at _runPendingImmediateCallback(isolate_patch.dart:120)
at _RawReceivePortImpl._handleMessage(isolate_patch.dart:185)

this exception needs to be handled in a better way

@jonahwilliams jonahwilliams added P2 Important issues not at the top of the work list c: crash Stack traces logged to the console tool Affects the "flutter" command-line tool. See also t: labels. labels Jun 7, 2021
@jonahwilliams
Copy link
Member Author

@bkonyi

@bkonyi bkonyi self-assigned this Jun 7, 2021
@bkonyi
Copy link
Contributor

bkonyi commented Aug 25, 2021

@jonahwilliams is this something we're still seeing?

@zanderso
Copy link
Member

zanderso commented Aug 31, 2021

Hi @bkonyi. Yes, this is currently the top crasher on the 2.5 beta. It is also the second most frequent crasher on the most recent dev 2.6.0-0.0.

@zanderso zanderso added dependency: dart Dart team may need to help us P2 and removed P2 Important issues not at the top of the work list labels Aug 31, 2021
@bkonyi
Copy link
Contributor

bkonyi commented Sep 1, 2021

I've started looking into this, but at this point I'm unable to reproduce, even after trying to force the error in a mocked environment. Are we 100% sure this is coming from shelf or its related packages? shelf request handlers are wrapped with a default error handler that responds to clients with a valid error message without actually resulting in an unhandled exception.

I've tried overriding the HTTP handler which forwards HTTP requests to the VM service with a handler that simply throws a ClientException, but that doesn't seem to result in any sort of unhandled exception in DDS.

@zanderso
Copy link
Member

zanderso commented Sep 1, 2021

Spoke with @jonahwilliams over chat. The shelf_proxy via dds is the one of two places the tool and its transitive dependencies use package:http. Specifically from here: https://github.com/dart-lang/shelf_proxy/blob/master/lib/shelf_proxy.dart. The other place is in crash reporting, but if the crash was in the crash reporting logic, then we wouldn't be seeing it in crash logging.

@bkonyi
Copy link
Contributor

bkonyi commented Sep 1, 2021

Ah okay, I think I see where the problem is. It looks like shelf only catches the unhandled asynchronous exceptions if it's in Zone.root. Since flutter_tools installs its own Zone somewhere in the stack, shelf assumes that Zone will handle the errors. Wrapping the invocation of io.serve(...) in DDS seems to fix the issue.

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Sep 1, 2021
handlers

Fixes flutter/flutter#84113

Change-Id: I416308845c5c7f9a1bb547b6429f1e9d49393583
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212268
Reviewed-by: Zach Anderson <zra@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
@zanderso zanderso added the waiting for PR to land (fixed) A fix is in flight label Sep 7, 2021
@zanderso
Copy link
Member

zanderso commented Sep 7, 2021

This is landing too late to cherry pick before 2.5 stable, but we will monitor the crash rate and decide if it should go into a hotfix release.

@zanderso
Copy link
Member

@christopherfujino @devoncarew This is the second most frequent CLI crasher on stable. I think we will need to get this queued up for a hotfix.

@christopherfujino
Copy link
Member

@bkonyi could you issue a dds point release (e.g. 2.0.4) for this? Currently, the 2.5 release is on dds: 2.0.3 https://github.com/flutter/flutter/blob/stable/packages/flutter_tools/pubspec.yaml#L13

@bkonyi
Copy link
Contributor

bkonyi commented Sep 15, 2021

Hotfix released in 2.0.4.

@devoncarew
Copy link
Member

Is the dds point release separate from the fix landed 9/8? (#89591)

@github-actions
Copy link

github-actions bot commented Oct 4, 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 -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2021
@flutter-triage-bot flutter-triage-bot bot added P0 Critical issues such as a build break or regression and removed P2 labels Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: crash Stack traces logged to the console dependency: dart Dart team may need to help us P0 Critical issues such as a build break or regression tool Affects the "flutter" command-line tool. See also t: labels. waiting for PR to land (fixed) A fix is in flight
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants