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

Initial connection with wrong IP address just hangs forever, no useful error message #6

Open
cstim opened this issue Oct 24, 2020 · 4 comments

Comments

@cstim
Copy link

cstim commented Oct 24, 2020

Use case: If you try to create a connection to a DCOM server (OPCDA Server in my example), but misspelled the IP address. Currently the socket setup just runs forever, but your library already has a timeout setting and the ComTransport also has an event listener to the 'error' event, but you have to do something there that actually signals the failed connection attempt to the client application.

I've filed a proposed solution in #5 which reliably returns an error if the initial connection attempt fails due to e.g. a non-existing IP address.

@steuck13
Copy link
Contributor

Greetings
Nice catch and thank you for the reminder. I will take a look at it on monday.

@cstim
Copy link
Author

cstim commented Nov 6, 2020

Any chance to have a look at this in the following days?

1 similar comment
@cstim
Copy link
Author

cstim commented Jan 20, 2021

Any chance to have a look at this in the following days?

@gfcittolin
Copy link
Member

Hi there. I'll be taking over the work on these nodes. I'm not as familiar with this code base as in other repos here, but from the changes proposed in the PR, i'd say they do make sense. We cannot just rely on OS events to assume the connection is broken.

What has caught my attention although is the handling of the timer created. Usually we need to clear the timer not only when the connection is successful (as it seems to be done there), but also when the object/instance is being destroyed/disconnected. I've seen some nasty bugs caused on race conditions like that. I'll need to find some time here to analyze the code path and setup an environment for some testing. I know this is months old already, but I'll need a bit more time to get this properly tested and fixed, sorry for that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants