Handling unix socket connection based on OS #21
Conversation
|
I had to merge another PR that moved some stuff around and now your PR conflicts with the |
|
I have resolved all the conflicts. |
src/client.js
Outdated
| this.connect(addr) | ||
|
|
||
| this.socket.on('error', function(err){ | ||
| // Writing to a file in case of error related to socket |
There was a problem hiding this comment.
In this case I'd rather throw an exception (that will be printed on the user's screen) and kill the app instead of writing to a file few people are aware of.
I don't know how Electron does it and I'm not really familiar with NodeJS, but when there's a fatal error in Electron, it prints the error in a nice window and exit the process. Could you do the same thing instead?
If this is not possible, you should only have to print the error in stderr which will then be picked up by the go side that will print it in its logs.
src/client.js
Outdated
| } | ||
|
|
||
| connect(addr) { | ||
| if (net.isIP(addr) > 0) { |
There was a problem hiding this comment.
I don't really like the idea of using net.isIP here.
Can you check whether simplifying to this works :
this.socket = net.createConnection(addr);If not, I'd rather see something along the lines of :
let u = url.parse(addr, false, false);
if (u.protocol === "tcp:") {
this.socket = new net.Socket();
this.socket.connect(u.port, u.hostname, function() {});
} else {
this.socket = net.createConnection(addr);
}There was a problem hiding this comment.
I tried with [below code], but there was some issue in windows with this I'll try again and update the code accordingly.
this.socket = net.createConnection(addr);
There was a problem hiding this comment.
Pardon me for late response, I tried testing client code with below function it works very well with darwin and linux but somehow does not work in windows. The app gets installed but does not runs so debugging that is taking a little time. I did try the other approaches (i.e "tcp" check as you and the isIP() check) but somehow windows app is not working.
this.socket = net.createConnection(addr);
Below are the two approaches I have used to build the app, code snippet of bundler.json
"environments": [
{"arch": "amd64", "os": "darwin"},
{
"arch": "amd64",
"os": "windows",
"env": {
"CC": "x86_64-w64-mingw32-gcc",
"CXX": "x86_64-w64-mingw32-g++",
"CGO_ENABLED": "1"
}
}
]
and
"environments": [
{"arch": "amd64", "os": "linux"},
{"arch": "386", "os": "windows"}
]
I need a bit more time to resolve this issue.
…nection() to create connection
|
@asticode |
|
@shrey-gang could you resolve conflicts with changes that were made in between ? It seems |
Conflict resolved |
Pull Request is partenered with asticode/go-astilectron#191
This will fix issue: asticode/go-astilectron#190