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

Debugger pauses on handled exceptions in async* methods as if they are unhandled #47985

Closed
aliyazdi75 opened this issue Dec 21, 2021 · 43 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. vm-debugger

Comments

@aliyazdi75
Copy link

This issue continues #37953 and #47692.

Regarding this issue for cached_network_image plugin, this problem still persists.

This is a reproducible Gist. As you can see, with defining an errorWidget, the exceptions should be handled but the debugger throws on cached_network_image exceptions only, not all flutter exceptions.

flutter doctor -v
[✓] Flutter (Channel unknown, 2.8.0, on Ubuntu 20.04.3 LTS 5.4.0-91-generic, locale en_US.UTF-8)
    • Flutter version 2.8.0 at /home/aliyazdi75/Documents/AndroidStudioProjects/flutter
    • Upstream repository unknown
    • Framework revision cf44000065 (12 days ago), 2021-12-08 14:06:50 -0800
    • Engine revision 40a99c5951
    • Dart version 2.15.0

[✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
    • Android SDK at /home/aliyazdi75/Android/Sdk
    • Platform android-31, build-tools 31.0.0
    • Java binary at: /home/aliyazdi75/Application/android-studio/jre/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at google-chrome

[✓] Linux toolchain - develop for Linux desktop
    • clang version 10.0.0-4ubuntu1
    • cmake version 3.16.3
    • ninja version 1.10.0
    • pkg-config version 0.29.1

[✓] Android Studio (version 2020.3)
    • Android Studio at /home/aliyazdi75/Application/android-studio
    • Flutter plugin version 63.2.1
    • Dart plugin version 203.8452
    • Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)

[✓] VS Code
    • VS Code at /snap/code/current
    • Flutter extension can be installed from:
      🔨 https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[✓] Connected device (3 available)
    • SM G955F (mobile) • ce031713bb10406e0d • android-arm64  • Android 9 (API 28)
    • Linux (desktop)   • linux              • linux-x64      • Ubuntu 20.04.3 LTS 5.4.0-91-generic
    • Chrome (web)      • chrome             • web-javascript • Google Chrome 96.0.4664.110

• No issues found!
@lrhn lrhn added the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label Dec 21, 2021
@a-siva
Copy link
Contributor

a-siva commented Dec 21, 2021

//cc @DanTup both of the above issue seems to be reported against VS code, could you help debug the issue and root cause it.

@a-siva
Copy link
Contributor

a-siva commented Dec 21, 2021

//cc @bkonyi

@aliyazdi75
Copy link
Author

@a-siva For me, it happened on intelliJ/AS, but I think it should be same as VS code.

@bkonyi
Copy link
Contributor

bkonyi commented Dec 21, 2021

If I had to guess, a Future somewhere is completing with an error and doesn't have an error handler associated with it. If that's the case, I don't think this is an SDK issue. Maybe @cskau-g has some ideas?

@DanTup
Copy link
Collaborator

DanTup commented Dec 21, 2021

@a-siva @bkonyi maybe related to yield*? I stripped it down to this:

import 'dart:async';

Future<void> main() async {
  try {
    await _updateFile().toList();
  } catch (e) {
    print('Handled: $e');
  }
}

Stream<String> _updateFile() async* {
  yield* _manageResponse();
}

Stream<String> _manageResponse() async* {
  throw 'error'; // The debugger will pause here, even though the error is going to be handled
}

As far as I can see, the error is handled (it prints "Handled" and there's no unhandled error written to stderr or non-zero exit code). I can't reproduce it without the yield*.

I don't know how to debug any further, but perhaps this simple repro helps :-)

@DanTup
Copy link
Collaborator

DanTup commented Dec 21, 2021

Actually, I can repro without yield* (though still with async*):

import 'dart:async';

Future<void> main() async {
  try {
    await _manageResponse().toList();
  } catch (e) {
    print('Handled: $e');
  }
}

Stream<String> _manageResponse() async* {
  throw 'error'; // The debugger will pause here, even though the error is going to be handled
}

@aliyazdi75
Copy link
Author

aliyazdi75 commented May 3, 2022

@DanTup Any updates on this?

@DanTup
Copy link
Collaborator

DanTup commented May 3, 2022

I'm not familiar with the internals of the debugger so I think this would need to be looked at by someone on the VM team.

@aliyazdi75
Copy link
Author

It's really annoying since we receive many issues on Sentry.

@mraleph
Copy link
Member

mraleph commented May 3, 2022

@aliyazdi75 how is this connected to Sentry?

@DanTup
Copy link
Collaborator

DanTup commented May 3, 2022

@aliyazdi75 I'm not familiar with Sentry, but if you're receiving runtime exception reports I think there may be two different issues here. My repro above is related to the debugger pausing on an exception that will be caught (despite being configured to only pause on uncaught exceptions). It only affects the debugger pausing and the error is otherwise handled correctly.

If you're seeing runtime errors from users apps, I think that's another issue (and one that you can resolve separately to the issue my repro above covers).

@aliyazdi75
Copy link
Author

aliyazdi75 commented May 3, 2022

@mraleph @DanTup We handled the situation in my repo for cached_network_image and it shows the widget when errorWidget triggers. But Sentry reports the exact issue that the debugger caught and paused. This is a sample issue on Sentry. I can confirm that the both are the exact same issue.

@mraleph
Copy link
Member

mraleph commented May 3, 2022

@aliyazdi75 this is not the same issue. this issue is about the debugger (or more specifically about interaction between exceptions from async* functions). Sentry does not use the debugger, so something else is wrong. If you have a reproduction please file a separate issue.

@aliyazdi75
Copy link
Author

@mraleph I don't think that's a separate issue. I reported what I observed for adding more information regarding this issue. I don't add misinformation.

I have no idea how Sentry report that async* function bug or even shouldn't do that. Although I'm sure that the debugger and Sentry report the same issue and I should probably ignore this Sentry issue again.

@mraleph
Copy link
Member

mraleph commented May 3, 2022

@aliyazdi75 again, I suggest you to review your code - these two issues are unrelated. Sentry installs a bunch of error listeners (e.g. runZoneGuarded, FlutterError.onError, etc). None of these listeners are affected by this issue which affect only debugger. So if Sentry reports an uncaught error this most certainly means that the error was in fact uncaught.

@derekxu16
Copy link
Member

Correct me if I'm wrong, but I think this is caused by #46318.

The debugger looks for exception handlers by checking the frames available in the stack trace

sdk/runtime/vm/debugger.cc

Lines 803 to 805 in 0304832

if (frame->HandlesException(exc_obj)) {
return frame;
}

A frame corresponding to main is not available when running Danny's example above with async* (#47985 (comment)). A frame corresponding to main is available when the example is modified to change _manageResponse from an async* function to an async one though.

This can be observed by running the VM with --trace_debugger_stacktrace and looking at the GetHandlerFrame: logs.

sdk/runtime/vm/debugger.cc

Lines 799 to 802 in 0304832

if (FLAG_trace_debugger_stacktrace) {
OS::PrintErr("GetHandlerFrame: #%04" Pd " %s", frame_index,
frame->ToCString());
}

@mraleph
Copy link
Member

mraleph commented Jun 23, 2023

@derekxu16 The failure on Danny's example it is not related to #46318, but you are correct that it related to awaiter stack not reaching the main.

The reason it does not reach main is because Stream.toList confuses the unwinder: it is a function that uses _Future._complete manually, so unwinder can't figure out where to look for the next frame.

This can either be fixed by rewriting toList to use async or by using @pragma('vm:awaiter-link') which I am working on (the prototype CL is here: https://dart-review.googlesource.com/c/sdk/+/299221)

@DanTup
Copy link
Collaborator

DanTup commented Jun 27, 2023

@mraleph do you think flutter/flutter#129121 could be related? See my comment at flutter/flutter#129121 (comment) for a simple repro and VM Service log.

@mraleph
Copy link
Member

mraleph commented Jun 27, 2023

@DanTup it's a similar issue. The actual exception is thrown when stack trace looks like this:

#0      _NativeSocket.lookup.<anonymous closure> (dart:io-patch/socket_patch.dart:520:26)
<asynchronous suspension>
#1      _NativeSocket.staggeredLookup.<anonymous closure>.lookupAddresses.<anonymous closure> (dart:io-patch/socket_patch.dart:632:39)
<asynchronous suspension>

staggeredLookup is an extremely complicated piece of async code - I am not even sure we can easily fix unwinding through it. I can check once I land support for awaiter-link.

@DanTup
Copy link
Collaborator

DanTup commented Jun 27, 2023

Got it, thanks! If I understand correctly, this means the proposal in flutter/devtools#5883 would also not help here (because the exception looks completely uncaught, rather than just being caught in non-debuggable file which that proposal would handle differently)?

copybara-service bot pushed a commit that referenced this issue Jun 30, 2023
The main contribution of this CL is unification of disparate
handling of various functions like `Future.timeout`,
`Future.wait`, `_SuspendState.createAsyncCallbacks` and
`_SuspendState._createAsyncStarCallback` into a single
`@pragma('vm:awaiter-link')` which allows Dart developers
to specify where awaiter unwinder should look for the next
awaiter.

For example this allows unwinding to succeed for the code like this:

    Future<int> outer(Future<int> inner) {
      @pragma('vm:awaiter-link')
      final completer = Completer<int>();

      inner.then((v) => completer.complete(v));

      return completer.future;
   }

This refactoring also ensures that we preserve information
(including Function & Code objects) required for awaiter
unwinding across all modes (JIT, AOT and AOT with DWARF stack
traces). This guarantees users will get the same information
no matter which mode they are running in. Previously
we have been disabling awaiter_stacks tests in some AOT
modes - which led to regressions in the quality of produced
stacks.

This CL also cleans up relationship between debugger and awaiter
stack returned by StackTrace.current - which makes stack trace
displayed by debugger (used for stepping out and determinining
whether exception is caught or not) and `StackTrace.current`
consistent.

Finally we make one user visible change to the stack trace:
awaiter stack will no always include intermediate listeners
created through `Future.then`. Previously we would sometimes
include these listeners at the tail of the stack trace,
which was inconsistent.

Ultimately this means that code like this:

    Future<int> inner() async {
      await null;  // asynchronous gap
      print(StackTrace.current); // (*)
      return 0;
    }

    Future<int> outer() async {
      int process(int v) {
        return v + 1;
      }

      return await inner().then(process);
    }

    void main() async {
      await outer();
    }

Produces stack trace like this:

    inner
    <asynchronous suspension>
    outer.process
    <asynchronous suspension>
    outer
    <asynchronous suspension>
    main
    <asynchronous suspension>

And when stepping out of `inner` execution will stop at `outer.process`
first and the next step out will bring execution to `outer` next.

Fixes #52797
Fixes #52203
Issue #47985

TEST=ci

Bug: b/279929839
CoreLibraryReviewExempt: CL just adds @pragma to facilitate unwinding
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-dwarf-linux-product-x64-try
Change-Id: If377d5329d6a11c86effb9369dc603a7ae616fe7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311680
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
@mattmyne
Copy link

mattmyne commented Aug 18, 2023

Just to add another example where this happens. Trying to use the flutter_map package from vscode in Windows (windows-x64) debug mode.

Created a new application using the "Flutter: New Project", Application (Empty), entered:
flutter pub add flutter_map
flutter pub add latlong2
and pasted the quickstart code from:
https://docs.fleaflet.dev/#demonstration

With only "uncaught exceptions" ticked I also get a SocketException "Failed host lookup: 'tile.openstreetmap.org'" "The requested name is valid, but no data of the requested type was found.", errno = 11004 in L520 of dart:io-patch\socket_patch.dart (with same stacktrace shown by @mraleph)

Also runs fine in both debug and run from flutter run command (and build). Unchecking both "All" and "Uncaught" breakpoints options in the debugger lets the program run without issue.

I'm using stable Flutter 3.13.0 (I've not tried on earlier versions)

Commented originally in issue flutter/flutter#129121

@DanTup
Copy link
Collaborator

DanTup commented Aug 24, 2023

I presume it is not related to the code in InternetAddress.tryParse which was throwing until this cherry pick was done #52487

I think it's related, but that CP was in the previous stable version and complaints in that version were that the debugger paused if "All Exceptions" was ticked. In last weeks stable, the debugger is pausing with only "uncaught exceptions" ticked which is significantly worse.

I've opened #53334 specifically for this issue to avoid confusing it with this one (which seems to be related to async*).

@DanTup DanTup changed the title Dart debugging throws handled exceptions as uncaught Dart debugging throws handled exceptions as uncaught in async* methods Aug 24, 2023
@laeo
Copy link

laeo commented Aug 31, 2023

This is really confusing me, i write a API call using Dio, and when i try to cal it without backend server running, vscode will pause my app and show me a uncaught exception SocketException.

@DanTup
Copy link
Collaborator

DanTup commented Aug 31, 2023

@laeo if this isn't in an async* method, it's probably not related to this issue.

There are some other similar open issues (such as #53334), although it's not clear to me what the expected behaviour is - if your backend server being down results in an uncaught exception, this sounds like behaviour I would expect.

If you think it's a bug (because something is catching the exception) and it doesn't seem like the same issue described in #53334, please file a new issue with a small (but complete) code example that can be used to reproduce the issue. Thanks!

@stealthyloon
Copy link

Is there still no fix for this?

@a-siva
Copy link
Contributor

a-siva commented Sep 1, 2023

Is there still no fix for this?

The fix has landed in the main branch and we should have a fix in stable via the hotfix release next week.

@DanTup
Copy link
Collaborator

DanTup commented Sep 4, 2023

@a-siva are you referring to the fix for #53334? Does that also solve this issue (related to async* functions)?

@YaKun-SH
Copy link

YaKun-SH commented Sep 7, 2023

Is there still no fix for this?

The fix has landed in the main branch and we should have a fix in stable via the hotfix release next week.

Will the next new stable version fix this issue?

@YDA93
Copy link

YDA93 commented Oct 31, 2023

Still exists in Flutter Channel stable, 3.13.9. the bug is annoying

@tomekit
Copy link

tomekit commented Nov 7, 2023

I can also confirm that this happen on stable, 3.13.9.

Here is my bug report with an example of code that can be used: cfug/dio#2025 but apparently there is not much Dio can do themselves to have it solved.

The uncaught exception issue starts in a function below dio-5.3.3/lib/src/dio_mixin.dart:

 @internal
  static Future<T> listenCancelForAsyncTask<T>(
    CancelToken? cancelToken,
    Future<T> future,
  ) {
    return Future.any([
      if (cancelToken != null) cancelToken.whenCancel.then((e) {
        throw e;
      }),
      future,
    ]);
  }

@DanTup
Copy link
Collaborator

DanTup commented Nov 7, 2023

@tomekit I'm not sure this is the same issue. This issue relates to the debugger pausing on an extension that will be caught, even if settings are such that only uncaught exceptions should pause (specifically in async* methods).

In your report, it sounds like the application behaviour is unexpected (and that nothing happens):

Nothing happens. Unexpected.

It's not clear to me if nothing is happening because the debugger has paused, or if you're just seeing unexpected runtime behaviour (that is unrelated to the debugger). If the latter, then I don't think it's the same issue.

Edit: I've updated the title to try and make it a little clearer

@DanTup DanTup changed the title Dart debugging throws handled exceptions as uncaught in async* methods Debugger pauses on handled exceptions in async* methods as if they are unhandled Nov 7, 2023
@HE-LU
Copy link

HE-LU commented Nov 8, 2023

I am experiencing completely the same issue. In my case, this happens using riverpod and dio. As for a sample code, I have the following UseCase:

@riverpod
Future<void> reportUserUseCase(ReportUserUseCaseRef ref, {required String userId}) async {
  final dio = ref.read(dioProvider);
  await dio.post('/users/report', data: '{userId: $userId}');
}

And then I call it like this somewhere in the code:

try {
  await ref.read(reportUserUseCaseProvider(userId: profileId).future);
} catch (e) {
  Fimber.e("Just sample error output");
}

Sadly, when there goes smth wrong on BE side, for example the endpoint does not exist, or return error, the debugger pauses on following line, even though the exception is catcher later on:

await dio.post('/users/report', data: '{userId: $userId}');

@DanTup
Copy link
Collaborator

DanTup commented Nov 8, 2023

@HE-LU can you provide a small complete sample that can be used to reproduce this?

If you're not inside an async* method (this is different to an async method without the *) then it's probably a different problem - in which case filing a new issue with a code sample to reproduce may be best.

@tomekit
Copy link

tomekit commented Nov 11, 2023

Hi @DanTup,

I'm not sure this is the same issue. This issue relates to the debugger pausing on an extension that will be caught, even if settings are such that only uncaught exceptions should pause (specifically in async* methods).

In your report, it sounds like the application behaviour is unexpected (and that nothing happens):

I've improved my report and wording little bit (cfug/dio#2025).
The issue with the debugger pausing is present in my example, so it's not exactly that nothing happens... it's just from an app perspective the request cancellation takes no effect.

Debugger:
Screenshot from 2023-11-11 11-04-29

It seems that I've incorrectly assumed that just because there is an issue with the debugger, it's also likely what "breaks" the cancellation (implemented by throwing exception from the async*)... and even though I wouldn't be surprised if these issues are somewhat related I probably shouldn't mix these two and derive any meaningful conclusions.

@tomekit
Copy link

tomekit commented Nov 11, 2023

Hi again @DanTup,

I've decided to play around and built an example to demonstrate an issue which I believe to be incorrect Dart behavior.
In summary, if an exception is thrown from the async*, it can't be caught with try/catch, also without try/catch it's not captured by the runZonedGuarded.onError, whereas uncaught exception thrown outside of async* gets registered correctly.

This issue is present regardless if I ran the app in debug/release mode and no debugger is involved here.

Please find buttons on the right side. The "red" ones indicate behavior which I consider not expected.
Screenshot from 2023-11-11 12-18-55
Full example: https://github.com/tomekit/dio_cancel_issue_stream_get/blob/main/lib/main.dart

I am running: Flutter 3.13.9 and running/building app on Ubuntu 23.04.

@DanTup
Copy link
Collaborator

DanTup commented Nov 11, 2023

@tomekit this issue is specifically about whether the debugger pauses execution and not about the runtime behaviour. If your application is not doing as you expect (for example if you "Start without Debugging" or disable breaking on exceptions), that is not the same issue.

That said, I think the few examples I checked in your repro are working as intended. For example:

FloatingActionButton(
  onPressed: () async {
    _manageResponse();
  },
  tooltip: 'Uncaught exception test from async* (no effect !)',
  child: const Icon(Icons.error, color: Colors.redAccent),
),

try/catch won't work on async methods unless you await the returned Future. You can see this with a simple sample here:

void main(List<String> arguments) async {
  try {
    foo(); // no await here
  } catch (e) {
    print('never executes');
  }
}

Future<void> foo() async {
  throw 'error';
}

If you remove async from the declaration of foo then the catch block will handle the error. The async keyword on the function changes the method from throwing an exception (synchronously) to instead returning a Future that completes with an error. A try/catch won't handle a function that returns a Future that completes with an error unless you await it.

@maxmitz
Copy link

maxmitz commented Dec 22, 2023

Is there any update here? It still happens in Flutter 3.16.5.

@thassio-vinicius
Copy link

I just had to downgrade all the way to Flutter 3.10.5 to get rid of this. This is directly affecting all error handling on Flutter's biggest HTTP package (Dio). Can we please get some traction on this issue?

@mraleph
Copy link
Member

mraleph commented Feb 15, 2024

The Dio cancellation exception problem was fixed by 0ea5013.

And the remaining cases will be fixed by https://dart-review.googlesource.com/c/sdk/+/322720

@thassio-vinicius
Copy link

@mraleph thank you! any idea on when should we expect this fix to be released?

@mraleph
Copy link
Member

mraleph commented Feb 15, 2024

@mraleph thank you! any idea on when should we expect this fix to be released?

The first one should already be on Flutter master channel - so you can test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. vm-debugger
Projects
None yet
Development

No branches or pull requests

17 participants