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

Dart debugging in Visual Code throws handled exceptions as uncaught #37953

Closed
cosinus84 opened this issue Aug 22, 2019 · 42 comments
Closed

Dart debugging in Visual Code throws handled exceptions as uncaught #37953

cosinus84 opened this issue Aug 22, 2019 · 42 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@cosinus84
Copy link

I think this a serious problem when working with dart/flutter.
flutter/flutter#33427

@vsmenon vsmenon added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Aug 22, 2019
@Kenji-K
Copy link

Kenji-K commented May 26, 2020

I know it's in poor taste to add a "me too" comment, but this seriously needs attention. I found my issue with flutter, but apparently it's not an isolated issue.

I'm attempting to run the following:

GoogleSignIn _googleSignIn = GoogleSignIn(
    scopes: <String>[
      'email'
    ],
  );

...

Future<FirebaseUser> _silentSignIn() async {
  try {
    _googleUser = await _googleSignIn.signInSilently(suppressErrors: false)
      .catchError((e) {
        print('$e');
      });

    final GoogleSignInAuthentication googleAuth = await googleUser.authentication;

    final AuthCredential credential = GoogleAuthProvider.getCredential(
      idToken: googleAuth.idToken, 
      accessToken: googleAuth.accessToken
    );

    _firebaseUser = (await _auth.signInWithCredential(credential)).user;
    print("signed in " + firebaseUser.displayName);
    notifyListeners();
    return firebaseUser;
  } catch (error) {
    print(error);
  }
  return null;
}

Trying both ways to catch the error, and I still get the breakpoint as if it was unhandled. I'm at my wits end with this.

image

@bwilkerson
Copy link
Member

@DanTup

@DanTup
Copy link
Collaborator

DanTup commented Jun 24, 2020

Just to be sure - in the Debug side bar in VS Code, you only have "uncaught exceptions" ticked, and not "all exceptions"?

(cc @zichangg)

@Kenji-K
Copy link

Kenji-K commented Jun 24, 2020

In my case, I have double checked that I have only uncaught exceptions ticked.

@DanTup
Copy link
Collaborator

DanTup commented Jun 25, 2020

@Kenji-K thanks!

@zichangg I can repro this using a clean flutter create project and replacing main.dart with the following (and adding google_sign_in: to the pubspec dependencies):

import 'package:flutter/material.dart';
import 'package:google_sign_in/google_sign_in.dart';

void main() {
  runApp(MyApp());
  _silentSignIn();
}

GoogleSignIn _googleSignIn = GoogleSignIn(scopes: <String>['email']);

Future<void> _silentSignIn() async {
  try {
    await _googleSignIn.signInSilently(suppressErrors: false).catchError(print);
  } catch (error) {
    print(error);
  }
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(title: 'Demo', home: Text('test'));
  }
}

It repros in both VS Code and Android Studio (though in Android Studio the editor goes to the source of the exception, whereas VS Code walks up the stack to the first frame that's user code). The source of the exception is here:

Screenshot 2020-06-25 at 10 49 09

As I understood it, the VM should see the catch block in the stack and know that it's handled. I don't know if it's related, but it looks like the exception is caught and rethrown in the frame above the user code:

Screenshot 2020-06-25 at 10 52 43

@DanTup
Copy link
Collaborator

DanTup commented Jun 25, 2020

(I can't edit labels, but this might be area-vm rather than area-analyzer?)

@bwilkerson bwilkerson added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. and removed area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Jun 25, 2020
@mehdiRezzagHebla
Copy link

From what I could gather, the dartlang team said that it is "impossible" for them to fix this, I am not an experienced dev and I don't need to be in order to know that that is not true, since other modern languages don't have this problem.

@mraleph
Copy link
Member

mraleph commented Nov 30, 2020

I would agree that this is indeed an unpleasant usability issue for the debugger. I don't see any reason though why we would not be able to make this work in common situations given that we should be able to peek through future chains.

In the code below:

import 'dart:async';

Future<void> baz() async {
  await null;
  throw 'A';
}

Future<int> bar() => baz().then((value) => 10);

Future<int> foo() async {
  try {
    await bar();
  } catch (e) {}
  return 0;
}

void main(List<String> arguments) async {
  await foo();
}

Debugger stops at throw as if it is uncaught.

We also get a really weird truncated lazy async stack trace with no position information in the last frame (/cc @mkustermann @cskau-g):

$ out/ReleaseX64/dart /tmp/uncaught.dart
#0      baz (file:///tmp/uncaught.dart:5:3)
<asynchronous suspension>
#1      bar.<anonymous closure> (file:///tmp/uncaught.dart)
<asynchronous suspension>

@mkustermann
Copy link
Member

Reason for truncation is probably usage of legacy baz().then((value) => 10 instead of value = await baz(); return 10. @cskau-g has recently added support for whenComplete() we could possibly extend that to then() as well.

Though generally speaking I think we should encourage users to use await and try/await/catch/finally instead of the pre-async/await then()/catchError()/whenComplete().

Also for certain cases it's not possible to select the right way to unwind, because a future can have multiple listeners, which would yield a back trace tree instead of a stack. We could possibly print an the tree, but that would bring in a whole lot of new issues (e.g. figure out nice way to print such a backtrace tree, making package:stack_trace understand it, ...)

@mraleph
Copy link
Member

mraleph commented Nov 30, 2020

@mkustermann If I replace then with timeout I get correct lazy async stack, but debugger still thinks exception is uncaught. I looked briefly at the code and it seems to attempt to make things work by looking for handler in the "awaiter stack trace" however this code is written in terms of causal stacks and has not been updated to use lazy async stacks at all.

There is a consideration here that there is a lot of legacy code (written without async/await) and code written in mixed style - sometimes hidden inside SDK libraries and packages which user did not author. I think we should definitely make things work for Future helper methods whenever we can (e.g. it is a bit puzzling that it would work for timeout but not catchError (BTW, replacing then with catchError produces somewhat garbled stack as well).

@DanTup
Copy link
Collaborator

DanTup commented Nov 30, 2020

Slightly related - this comes up quite a bit in the Dart-Code issue tracker, and I've generally been encouraging people to use try/catch where possible to avoid it (although often they're calling libraries that they can't easily modify), though some examples came up where this isn't possible, for example:

return FutureBuilder<Widget>(
  future: f,
  builder: (BuildContext context, AsyncSnapshot<String> snapshot) {
    if (snapshot.hasData) {
      return SuccessfulWidget();
    } else if (snapshot.hasError) {
      return ErrorMessageWidget(snapshot.error);
    }

    return LoadingSpinnerWidget();
  }
);

Here, the code that's handling the error is in FutureBuilder so it can't easily be converted to use try/catch. If I understand the suggestion above of "peeking through future chains", I think that would also handle this (as it's likely the error handler has been attached before the code in the Future throws)?

@ghost
Copy link

ghost commented Dec 3, 2020

There a bit of back-and-forth here, but we have no assignee and no clear action items, AFAICT.
@mraleph and/or @mkustermann, can you clarify what we'd like to do on this issue, and whether it's something I need to look into?

@mkustermann
Copy link
Member

Regarding the truncated stack: We can investigate adding support for the legacy Future.then() and Future.catchError() APIs as part of the existing open issue here: #40815

@mkustermann mkustermann assigned ghost Dec 3, 2020
@mkustermann
Copy link
Member

@cskau-g Could you investigate the uncaught error reporting functionality - both when it is actually caught (as this issue) as well as reporting exceptions as uncaught if the handler was specially marked (see flutter/flutter#17007)

@mkustermann mkustermann added this to the January 2021 milestone Dec 18, 2020
@franklinyow
Copy link
Contributor

@cskau-g @mkustermann Any update?

@mraleph
Copy link
Member

mraleph commented Jan 12, 2021

@cskau-g has landed changes on our side. The ball is now in Flutter court.

@mraleph mraleph closed this as completed Jan 12, 2021
@DanTup
Copy link
Collaborator

DanTup commented Jan 12, 2021

@mraleph are the changes different to the ones in #17007? Although this sounds like the same issue, I believe it's actually the opposite problem.

#17007 is about exceptions that Flutter is catching, but are logically uncaught (eg. expected behaviour is for the debugger to break).

This issue is about exceptions that are being caught (eg. with .catchError()) but the debugger considers uncaught (because there's no catch in the stack?) and therefore result in the debugger breaking (making the user believe there is an unhandled error they need to do something about), but clicking Continue results in the app continuing to work as expected.

@mraleph
Copy link
Member

mraleph commented Jan 12, 2021

My mistake, yeah it is a separate (mirror) issue. I got confused.

@mraleph mraleph reopened this Jan 12, 2021
@ghost
Copy link

ghost commented Jan 26, 2021

mraleph and I agreed to push this off this milestone to avoid rushing the change in, seeing how it's already gotten reverted twice.

The follow-up fix for the .catchError will likewise not be in this milestone.

dart-bot pushed a commit that referenced this issue Jan 26, 2021
TEST=ASAN; Various 'causal' tests updated below.

Issues addressed in this revision:
- #44708 ASAN
- #44700 SegFault

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-asan-linux-release-x64-try,vm-kernel-asan-linux-release-x64-try,analyzer-linux-release-try,analyzer-analysis-server-linux-try,analyzer-nnbd-linux-release-try
Bug: #40815, #37953
Change-Id: I8b8f6ee2e5d4ca2e6bea988ec1cd9f912ddf8240
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180186
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
dart-bot pushed a commit that referenced this issue Feb 2, 2021
TEST=Added new Future.catchError pause-on-unhandled-exceptions test.

Bug: #37953
Change-Id: Ieb3cf834eacad4f9b98ab2db0e5813e9bfa09747
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180840
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
@ghost
Copy link

ghost commented Feb 2, 2021

With dfd5413 the last piece of the puzzle should now be in place.

Closing this issue here. But as always, feel free to reopen if you encounter any further issues with the above cases.

Thank you.

@ghost ghost closed this as completed Feb 2, 2021
@DanTup
Copy link
Collaborator

DanTup commented Feb 11, 2021

@cskau-g seems to work great, thank you!

@edvinasgestautas
Copy link

hey sorry for a newbie question but do you have any idea when will flutter be built with this fix ? or how can I check it ?

@kevinmarrec
Copy link

@DanTup I tried with dev channel of Flutter but it seems the issue is still here.

Maybe last flutter dev build doesn't use last dart sdk dev which includes the fix ?

@mraleph
Copy link
Member

mraleph commented Feb 25, 2021

@kevinmarrec dev contains dfd5413. Could you describe the issue you are seeing in more details? We added support for .then and .catchError for futures, but it might be that we are still missing support for some other pattern (we are playing a bit of whack-a-mole here).

@kevinmarrec
Copy link

kevinmarrec commented Feb 25, 2021

@mraleph

When debugging this code on VS Code (F5) with only Uncaught Exceptions checked :

import 'package:http/http.dart' as http;

Future<void> main() async {
  try {
    await http.get(Uri.parse('www.fail.com'));
  } catch (err) {
    debugPrint('Error caught');
  }
}

VS Code still breaks (breakpoint) in third party (http here) cause it thinks the error hasn't been caught, but I do (catch) :

image

Using catchError doesn't work as well :

import 'package:http/http.dart' as http;

Future<void> main() async {
  http.get(Uri.parse('www.fail.com')).catchError((error) {
    debugPrint('Error caught');
  });
}
$ dart --version
Dart SDK version: 2.13.0-30.0.dev (dev) (Fri Feb 12 04:33:47 2021 -0800) on "windows_x64"

@mraleph
Copy link
Member

mraleph commented Feb 25, 2021

Yeah, I can see it does not work. It seems if you have more than one Future in the chain things break.

Future<void> throwAsync() async {
  await Future.delayed(const Duration(milliseconds: 100));
  throw 'Throw from throwAsync!';
}

Future<void> foo() async {
  await Future.delayed(const Duration(milliseconds: 100));
  await throwAsync();
}

Future<void> main() async {
  try {
    await throwAsync();  // Ok.
  } catch (err) {
    print('Error caught');
  }
  try {
    await foo();  // Not okay.
  } catch (err) {
    print('Error caught');
  }
}

@mraleph mraleph reopened this Feb 25, 2021
@kevinmarrec
Copy link

kevinmarrec commented Feb 25, 2021

@mraleph Good catch !

I can indeed confirm it breaks when there's more than one Future in the chain.

It will be my pleasure to test/try again when there's a fix coming up :)

And it would be awesome if it got fixed before next Dart release lands :)

@ghost
Copy link

ghost commented Feb 25, 2021

I'll have a look.

@namchuai
Copy link

namchuai commented Mar 7, 2021

Sorry for asking an obvious question here. When can we get this fix included in dart sdk for fluttter?
AFAIK, we cannot specify a dart sdk version with flutter sdk (https://stackoverflow.com/a/51514523/4700001).

@kevinmarrec
Copy link

kevinmarrec commented Mar 7, 2021

@namchuai I think we need to wait Dart 2.13 release and then most probably Flutter 2.1 will feature Dart 2.13

It may also be available on a beta Flutter channel in the coming weeks (may need to ask Flutter team to see if it can land quicker)

@mraleph
Copy link
Member

mraleph commented Mar 8, 2021

@namchuai to expand on @kevinmarrec response. Dart's master branch is automatically continuously being rolled into Flutter's master branch. So if you are using master channel then you should already have a fix included (unfortunately at the moment that is the only channel of Flutter that includes the fix). All other branches are entirely aligned as well (e.g. beta channel of Flutter uses beta channel of Dart and the same for stable channel). Except for very serious bugs (e.g. crashes) there is no way to expedite this process - it happens at a fixed cadence.

@a7md0
Copy link

a7md0 commented Nov 12, 2021

Still having this issue with 3rd party package (flutterchina/dio)

$ flutter --version
Flutter 2.5.3 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 18116933e7 (4 weeks ago) • 2021-10-15 10:46:35 -0700
Engine • revision d3ea636dc5
Tools • Dart 2.14.4

@mraleph
Copy link
Member

mraleph commented Nov 13, 2021

@a7md0 please file a new issue and provide a reproduction. just saying "Still having this issue" is unactionable for us, because we have fixed all cases we could reproduce - so your issue is likely something else.

@a7md0
Copy link

a7md0 commented Nov 13, 2021

@a7md0 please file a new issue and provide a reproduction. just saying "Still having this issue" is unactionable for us, because we have fixed all cases we could reproduce - so your issue is likely something else.

#47692

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, and the AOT and JIT backends. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests