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

Add remote port information to Socket errors #12693

Open
whesse opened this issue Aug 23, 2013 · 30 comments
Open

Add remote port information to Socket errors #12693

whesse opened this issue Aug 23, 2013 · 30 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io P2 A bug or feature request we're likely to work on triaged Issue has been triaged by sub team type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@whesse
Copy link
Member

whesse commented Aug 23, 2013

Errors thrown by Socket.connect currently print the local host and local port information, rather than the remote host and port.

To fix this, we would need to print both, since in the case of a socket created by accept on a listening socket, we want to know the local host and port, and there is no difference between a socket created by listening and one created by connect.

The _NativeSocket class has a cached copy of the local port, but the remote port information can only be gotten from the nativeGetRemotePeer call, which will fail if a connection has failed, like by connection refused.

So to implement this, we would need a remote port field in NativeSocket, which is filled in when a connect attempt is made, or when an accept is performed. Then we could report this in errors.

@andersjohnsen
Copy link

Set owner to @skabet.

@sgjesse
Copy link
Contributor

sgjesse commented Sep 25, 2013

Removed the owner.

@sgjesse
Copy link
Contributor

sgjesse commented Oct 28, 2013

Added this to the Later milestone.

@sgjesse
Copy link
Contributor

sgjesse commented Nov 5, 2013

Issue #14783 has been merged into this issue.

@andersjohnsen
Copy link

I'll look into sorting this out, making it less confusing.


Set owner to @skabet.
Added Started label.

@kevmoo
Copy link
Member

kevmoo commented May 14, 2014

Removed Area-IO label.
Added Library-IO, Area-Library labels.

@andersjohnsen
Copy link

Removed the owner.
Added Triaged label.

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@whesse whesse added Type-Enhancement P3 A lower priority bug or feature request library-io area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Aug 4, 2014
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed triaged labels Feb 29, 2016
@Hixie
Copy link
Contributor

Hixie commented Nov 28, 2016

It seems that for some of the messages, it's actually the local port and the remote host name, which is a significant source of confusion when debugging socket-based communications. (For a while I thought my program had a bug where I was setting the wrong remote port.)

@mleonhard
Copy link

I just wasted an hour because of this issue. How about bumping its priority?

@sortie
Copy link
Member

sortie commented Jun 20, 2019

Thanks for triaging duplicate issues. This bug is a favorite pet peeve of mine. I'm working on a fix as part of a breaking change.

@ulidtko
Copy link

ulidtko commented Jun 24, 2019

Well... being reported in 2013, almost 6 years ago, this bug is graduating into a pet dragon peeve I guess.

For anyone having the capacity to use their own build of Dart SDK, PR #36038 (unmerged) has a single-line tweak that you can cherry-pick and get the correct port in logs (for client sockets at least). Without any breaking changes.

@Kaushal0709
Copy link

Well... being reported in 2013, almost 6 years ago, this bug is graduating into a pet dragon peeve I guess.

For anyone having the capacity to use their own build of Dart SDK, PR #36038 (unmerged) has a single-line tweak that you can cherry-pick and get the correct port in logs (for client sockets at least). Without any breaking changes.

We are also facing the same problem in flutter 2.4.0. Can you please help me to resolved this if it is only one line fix.

Regards,
Kaushal

@ulidtko
Copy link

ulidtko commented Aug 1, 2019

@Kaushal0709,

We are also facing the same problem in flutter 2.4.0. Can you please help me to resolved this if it is only one line fix.

It's here: 2acc8a4

You will need to cherry-pick that commit into your Dart SDK source (or just edit in that line manually), then build Dart SDK from the patched source, and use that in your Flutter distribution.

The process is exactly the same as building Dart SDK from source, except that you do one extra step between Getting the source and Building, namely: patching the source.

@sortie
Copy link
Member

sortie commented Dec 6, 2019

I have a proposed fix at https://dart-review.googlesource.com/c/sdk/+/127465.

@ulidtko
Copy link

ulidtko commented Dec 19, 2019

Rebased patch for 2.7.0, until the breaking change lands into a release:

From: Max <ulidtko@gmail.com>
Date: Thu, 19 Dec 2019 16:19:07 +0000
Subject: [PATCH] Fix wrong port in Dart SocketError messages

See https://github.com/dart-lang/sdk/issues/12693
and https://github.com/dart-lang/sdk/pull/36038

Apply to: dart-sdk 2.7.0 (git 4cc36055d6803b899667feaedc1216a96e9d1c72)

---
 runtime/bin/socket_patch.dart | 1 +
 1 file changed, 1 insertion(+)

diff --git c/sdk/lib/_internal/vm/bin/socket_patch.dart i/sdk/lib/_internal/vm/bin/socket_patch.dart
index 5771c76909..c0b9041b4f 100644
--- c/sdk/lib/_internal/vm/bin/socket_patch.dart
+++ i/sdk/lib/_internal/vm/bin/socket_patch.dart
@@ -499,6 +499,7 @@ class _NativeSocket extends _NativeSocketNativeWrapper with _ServiceObject {
         final _InternetAddress address = it.current;
         var socket = new _NativeSocket.normal();
         socket.localAddress = address;
+        socket.localPort = port;
         var result;
         if (sourceAddress == null) {
           result = socket.nativeCreateConnect(

@sortie
Copy link
Member

sortie commented Dec 19, 2019

The above patch is not correct as the problem is deeper (the whole localAddress and localPort variable semantics were wrong) and IIRC might actually cause other issues. If one is willing to cherry-pick a patch anyway, one can apply: https://dart-review.googlesource.com/c/sdk/+/127465

I'm currently quantifying how potentially breaking the fix is (changing the semantics of .address, the port fix itself is non-breaking, but I bundled them together) (it's probably not very breaking, but we are careful). I've done a semantic analysis of all pub packages and need to review the results. I also need to review other internal source code likewise. I'll do that once I'm back in the new year, and then I'll proceed to file a proper breaking change request, fix a couple tests, and get the change landed.

@AbeerSul
Copy link

none of this actually worked for me :/
this command worked like magic "adb reverse tcp:5000 tcp:5000" 💯

@Kushagrasri
Copy link

i tried everything, working on emulator, android, but the same issue appeared all the time.
even tried adb reverse tcp:port tcp:port
then i used an application called ngrok which redirects to your localhost:port. still, i was getting the same error. then, i disabled windows firewall and eureka!!!!
hope this helps someone.

@davidmorgan
Copy link
Contributor

+1 to the wrong (local) port being shown on connection failures--this cost us some engineering time this week.

Good luck on the fix ;) from my point of view, not too worried about wrong semantics that have been there for a long time--just the port in the error message would have saved us. Thanks.

@a-siva
Copy link
Contributor

a-siva commented Nov 12, 2021

@sortie do you have cycles to take your CL thru the review and breaking change process and land it or do you want somebody else to takeover and work on it ?

@a-siva a-siva added triaged Issue has been triaged by sub team and removed library-io-triaged labels Dec 20, 2022
@matheszabi
Copy link

It is still not fixed!!! Why???

final channel = ClientChannel('localhost',
        port: 8080,
        options:
        const ChannelOptions(credentials: ChannelCredentials.insecure()));

Caught error: gRPC Error (code: 14, codeName: UNAVAILABLE, message: Error connecting: SocketException: Connection refused (OS Error: Connection refused, errno = 111), address = localhost, port = 37674, details: null, rawResponse: null, trailers: {})

The 37674 is randomly changing

I have no idea why can't get to port 8080, since:


    await server.serve(port: 8080);
    print('Server listening on port ${server.port}...');

Server listening on port 8080...

@a-siva
Copy link
Contributor

a-siva commented Mar 4, 2024

@brianquinlan could we take over this CL https://dart-review.googlesource.com/c/sdk/+/127465 and take it through the breaking change process and land it.

@ulidtko

This comment was marked as off-topic.

@brianquinlan
Copy link
Contributor

I had a PR to do this: https://dart-review.googlesource.com/c/sdk/+/272520

But I couldn't get the windows tests to pass. Let me take a look at it again.

@brianquinlan
Copy link
Contributor

@matheszabi please follow the Dart Code of Conduct and behave respectfully.

@matheszabi

This comment was marked as abuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io P2 A bug or feature request we're likely to work on triaged Issue has been triaged by sub team type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests