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
Comments
Set owner to @skabet. |
Removed the owner. |
Added this to the Later milestone. |
Issue #14783 has been merged into this issue. |
I'll look into sorting this out, making it less confusing. Set owner to @skabet. |
Removed Area-IO label. |
Removed the owner. |
Removed this from the Later milestone. |
Removed Oldschool-Milestone-Later label. |
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.) |
I just wasted an hour because of this issue. How about bumping its priority? |
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. |
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, |
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. |
I have a proposed fix at https://dart-review.googlesource.com/c/sdk/+/127465. |
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(
|
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. |
none of this actually worked for me :/ |
i tried everything, working on emulator, android, but the same issue appeared all the time. |
+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. |
@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 ? |
It is still not fixed!!! Why???
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:
Server listening on port 8080... |
@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. |
This comment was marked as off-topic.
This comment was marked as off-topic.
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. |
@matheszabi please follow the Dart Code of Conduct and behave respectfully. |
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.
The text was updated successfully, but these errors were encountered: