[CP] Do not throw during InternetAddress.tryParse

[brianquinlan]

Commit(s) to merge

14cb98e

Target

stable

Prepared changelist for beta/stable

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

Issue Description

The issue is that some users are debugging their applications with "Break on all exceptions" enabled. Dart 3.0 changed the behavior of HttpClient so that InternetAddress.tryParse is used. This method uses exceptions as part of its implementation, which provokes a debugger break that is confusing to some users.

This occurs on all platforms.

What is the fix

The fix is to not use exceptions internally in InternetAddress.tryParse.

Why cherry-pick

The reason to not cherry pick this PR is because the original implementation is correct.

But, despite heroic attempts by @DanTup, some users were unconvinced and fixing this ASAP seems like the most practical thing to do.

Risk

low

Issue link(s)

flutter/flutter#126650, #52423

Extra Info

No response

[itsjustkevin]

@a-siva could you take a look at this cherry-pick?

@brianquinlan do you see this as a must hotfix?

[brianquinlan]

@itsjustkevin Lots of users seem to have been confused by this issue but certainly it is possible for people to work without the fix.

[a-siva]

People can workaround without the fix but the comments on the issue seem to indicate that people view it as a change from the previous version and seem to think the app is crashing.
The workaround is to have people not use 'break on all exceptions' and from the comments it seemed like it was not a good option for the developers.

[a-siva]

the change lgtm.
Based on the comments in the original issue feel it would improve developer experience, @jacob314 could you chime in on whether it makes sense to cherry pick this change into the hotfix.

[yelmuratoff]

@brianquinlan I installed latest flutter version 3.10.2 with Dart 3.0.2 and this error not fixed(((

[itsjustkevin]

@K1yoshiSho this change is not yet in Dart or Flutter.

[brianquinlan]

@K1yoshiSho When I suggested cherrypicking this for Dart 3.0.2, it had not been released yet and I thought that there was a chance to get it in. My mistake.

[itsjustkevin]

Friendly bump @jacob314

[jacob314]

This should be hot fixed due to how much it improves the developer experience. A lot of developers depended on "pause on all exceptions" and throwing + catching on InternetAddress.tryParse made that workflow unusable.

[DanTup]

There appears to be some more confusion here. It looks like this change was merged into 3.0.3 although this issue wasn't closed. The note on the release notes also seems to have a copy/paste error and links to a different issue.

People have upgraded and are reporting the issue still exists, but I just tested this and I'm hitting a different caught exception now using the same code:

import 'dart:io';

import 'package:flutter/material.dart';

main() async {
  var client = HttpClient();
  var req = await client.openUrl("get", Uri.parse("https://www.bbc.co.uk"));
  var resp = await req.close();
  print(resp.statusCode);

  return runApp(const MaterialApp(
      home: Scaffold(
    body: Center(child: Text("Hello")),
  )));
}

WThe error suggests it failed to resolve www.bbc.co.uk, but that's definitely a valid host that resolves. Not pausing on caught exceptions works fine and the request completes (I see a 200 status code printed).

This isn't the same problem (it's a different exception), but the cherry-pick here unfortunately hasn't changed anything as far users see - there is still a caught exception here, it just happens to be a slightly different one.

I've opened #52615 to keep this separate (since it's a different issue). I guess the arguments above still apply to fixing/cherry-picking it, although I would still like to encourage users to use "Break on uncaught exceptions" and provide feedback if they feel they need to use "Break on all exceptions" because there's probably something else not working as users want here too.

[milindgoel15]

I'm just wondering why Flutter even throws the exception when it clearly completes the request with a status code of 200. The exception should not occur in the first place, whether the "Break on all exceptions" is ticked or not. I mean should Flutter really throw the exception on valid hosts as well?

Then changing the throw statement to returning the exception would not be required.

[DanTup]

It's probably better to have any discussion about that issue on #52615 since this issue was related to a cherry-pick for the original issue. I think one of the original changes was related to IPv6+IPv4 though, so I wonder if it simultaneously tries to resolve both (unfortunately the call stack isn't that useful here because of the async code/callbacks).

@itsjustkevin should this one be closed since the change in the request shipped even if there might need to be another fix?

[IncognitoGK9]

This issue is still there with the latest updates. If one unchecks the Breakpoints: All Exceptions and Uncaught Exceptions, then the InternetAddress check works for real and emulator devices. While thte unchecking is not the right way, the throw on network check is causing multiple breakpoints if one is using a network resource.

[DanTup]

There is a similar issue to this one at #53334 which has been fixed and there's a cherry-pick request open at #53450.