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

Fix missing error if initial connection cannot be established. #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cstim
Copy link

@cstim cstim commented Oct 10, 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 we already have a timeout setting and also an error event which we only have to use.

Proposed solution: Add a timer that lets the ComTransport object emit an error event if the timeout times out before the socket has connected, otherwise clear the timeout again immediately.

@steuck13
Copy link
Contributor

steuck13 commented Oct 28, 2020

Greetings
The idea here is to be able to close the socket in case of connecting to the wrong address? If so, the chain of function calls that end on calling the close function from the ComTransport class doesn't serve this purpose?

If I'm not mistaken if the socket is unable to connect it will emit an error event which is transformed and emitted as a 'disconnected' one which can then be used to manually call the close.

@cstim
Copy link
Author

cstim commented Oct 30, 2020

Well, the error is the following: When I set up a connection to an OPC-DA server according to your instructions https://github.com/st-one-io/node-dcom, my example code looks as follows:

`
const domain = 'WORKGROUP'
const username = 'myuser'
const password = 'mypassword'
const ipAddress = '1.2.3.4'
const classIdString = 'F8582CF2-88FB-11D0-B850-00C0F0104305' // Matrikon.OPC.Simulation

const sessionSingleton = new Session()
const comSession = sessionSingleton.createSession(domain, username, password)
const clsid = new Clsid(classIdString)
let comServer = new ComServer(clsid, ipAddress, comSession)

// start the COM Server
await comServer.init() // this promise hangs (=does not resolve or reject) upon wrong ipAddress
console.log('Successfully connected')
`
(this example is also submitted as PR #8 because this should exist as an example in the codebase)

In this code, the line comServer.init() will hang forever if the IP Address is an unreachable address, for example someone has mistyped the address.

I am asking for getting a useful error message if the address is unreachable. My proposed patch here is one solution I could come up with: Upon unreachable IP address, the ComTransport just doesn't return (=resolve), but the usual behaviour is to have a timeout running in parallel. So I added this timeout in ComTransport to let it emit an 'error' signal, upon which the Rpc Stub also throws an error.

There might be other solutions possible, of course. My point is that the current code still doesn't handle the unreachable IP address situation and this should be improved. If you already have a different possibility to catch this situation, my request probably asks for documenting this possibility and I'll happily use that. Thanks!

@steuck13
Copy link
Contributor

Greetings
You can add a listener to comServer

mycomserver.on("disconnected", function() {

});

However I made a mistake by not including the error message so you can listen to an event but it will say nothing about what happened. Also the time we'll wait for the socket cannot be changed so your suggestion seems like a good solution . I will play with it a bit and if everything is alright this can be merged.

Ty

Christian Stimming added 2 commits November 27, 2020 08:35
E.g. if the IP address of the server is wrong: Currently the socket setup
just runs forever, but the situation of a wrong server address should
better be handled somehow.
@cstim
Copy link
Author

cstim commented Nov 27, 2020

Any news on including this PR here? You mentioned the 'disconnected' event; however, even though this event exists, it does not get emitted when the initial connection cannot be established (such as wrong IP address). See the provided example file in PR #8: If that one is started with any IP address which doesn't point to an OPC-DA server, currently the call comServer.init() just hangs forever (edit: not forever, but for something like 2-3 minutes), regardless of whether the event is subscribed to or not.

What else should I provide to you? I submitted a bug description including steps on how to reproduce; I added an example file that directly shows the erroneous behaviour (in PR #8); I submitted a PR here that fixes this. What else do you want to receive?

@cstim
Copy link
Author

cstim commented Jan 20, 2021

Any news on including this PR here?

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

Successfully merging this pull request may close these issues.

2 participants