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

[CP] [IO] Use try/catch in lookupAddresses instead of Future error #53450

Closed
a-siva opened this issue Sep 6, 2023 · 13 comments
Closed

[CP] [IO] Use try/catch in lookupAddresses instead of Future error #53450

a-siva opened this issue Sep 6, 2023 · 13 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve

Comments

@a-siva
Copy link
Contributor

a-siva commented Sep 6, 2023

Commit(s) to merge

f7ef526

Target

stable 3.1

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/324569

Issue Description

With the Dart 3.1 release a number of Flutter users are facing issues with running their app in the debugger (produces an unhandled exception). For details please see discussion in #53334

What is the fix

This change fixes lookupAddress to use a try/catch instead of returning a Future error, this exception is the one that users where hitting often and with this fix they won't run into it.
The actual fix for the issue has landed in main but is a large change and does not make sense to cherry pick into stable at this point.

Why cherry-pick

A number of Flutter users have run into this issue.

Risk

low

Issue link(s)

#53334

Extra Info

No response

@a-siva a-siva added the cherry-pick-review Issue that need cherry pick triage to approve label Sep 6, 2023
@a-siva
Copy link
Contributor Author

a-siva commented Sep 6, 2023

//cc @mraleph for lgtm

@devoncarew devoncarew added the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label Sep 6, 2023
@itsjustkevin itsjustkevin added the cherry-pick-approved Label for approved cherrypick request label Sep 11, 2023
@itsjustkevin
Copy link
Contributor

Approval by @aam on the CL.

copybara-service bot pushed a commit that referenced this issue Sep 11, 2023
Applying patch from @aam for using try/catch in lookupAddresses instead
of Future error.

This cherry pick into the stable branch is to ensure we
address the expedient issue in #53334

TEST=new test added.

Bug: #53334
Change-Id: Ia5375cbd118d8d6437cf6869383f173cab48aa3f
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/323684
Cherry-pick-request: #53450
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324569
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
@DanTup
Copy link
Collaborator

DanTup commented Sep 12, 2023

Out of interest, do new Dart stables automatically get rolled into Flutter stable, or do additional issues need creating to cherry-pick them? I looked on https://github.com/flutter/flutter/wiki/Flutter-Cherrypick-Process but it doesn't seem to describe how Dart cherry-picks get to Flutter.

@athomas
Copy link
Member

athomas commented Sep 12, 2023

The releases are generally* aligned and Flutter picks up all Dart releases (except for the Dart dev channel). Technically, Dart releases are cherry-picked into Flutter releases but it's part of the process and doesn't need additional requests.

*We can do a Dart-only release but we'd only do it if Flutter is definitely unaffected by the changes. Probably, happens only every few years (I only remember one example).

@DanTup
Copy link
Collaborator

DanTup commented Sep 12, 2023

Got it, thanks for the explanation :-)

@fisforfaheem
Copy link

thanks

@escamoteur
Copy link

I upgraded to 3.13.4 and still have the breaking of the debugger when 'break on uncaught exceptions' is enabled

@a-siva
Copy link
Contributor Author

a-siva commented Sep 14, 2023

I upgraded to 3.13.4 and still have the breaking of the debugger when 'break on uncaught exceptions' is enabled

Can you share the backtrace of the exception that is showing up, it is possible that you are hitting a different exception than the most common one that folks were hitting with lookupAddress. This cherry pick only addresses that case.

@mihalycsaba
Copy link

did this fix the exception issue with the http package? dart-lang/http#1008

@a-siva
Copy link
Contributor Author

a-siva commented Sep 15, 2023

did this fix the exception issue with the http package? dart-lang/http#1008

Yes

@escamoteur
Copy link

if I enabale `break on all execptions´I still get this here
grafik
which I honestly don't understand as the host name is valid, so why would it throw an exception at all

after a flutter clean I now at least can use the setting break on uncaught execptions

@menghui-hata
Copy link

image
I am still getting this error with dio package after upgrading to 3.13.4

@a-siva
Copy link
Contributor Author

a-siva commented Sep 19, 2023

@menghui-hata and @escamoteur the cherry pick done was a work around for the specific exception issue with the staggeredLookup method in dart:io library that a number of folks were hitting, the real fix for the problem is in the main branch (the changes were too many to cherry pick back to the stable channel).

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. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve
Projects
None yet
Development

No branches or pull requests