error: connect ECONNREFUSED 127.0.0.1:port for request inside exit event listener for v6.1.0

[riddheshMarkandeya]

I'm trying to send some request on the exit event.

const syncRequest = require('sync-request');
console.log('Process started');

process.on('exit', () => {
  try {
    let res = syncRequest('GET', 'https://jsonplaceholder.typicode.com/users/1');
    console.log(JSON.parse(res.getBody()));
  } catch(err) {
    console.log('Got error');
    console.log(err);
  }
});

throw new Error('Boo');

This works for the sync-request version 4.0.0 but fails for the 6.1.0.

Error for 6.1.0:

Process started
Got error
Error: nodeNC failed:

events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: connect ECONNREFUSED 127.0.0.1:62440
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1191:14)
    at sendMessage (C:\work\winston-test\node_modules\sync-rpc\lib\index.js:146:13)
    at C:\work\winston-test\node_modules\sync-rpc\lib\index.js:175:25
    at request (C:\work\winston-test\node_modules\sync-request\lib\index.js:28:15)
    at process.on (C:\work\winston-test\sync-request-test.js:6:15)
    at emitOne (events.js:121:20)
    at process.emit (events.js:211:7)
    at process._fatalException (bootstrap_node.js:399:21)
C:\work\winston-test\sync-request-test.js:14
throw new Error('Boo');
^
Error: Boo

Expected output as per 4.0.0:

Process started
Could not use "nc", falling back to slower node.js method for sync requests.
{ id: 1,
  name: 'Leanne Graham',
  ...
}
C:\work\winston-test\sync-request-test.js:10
throw new Error('Boo');
^
Error: Boo

Is child process getting killed before it's able to make the request? Not sure why it would work for the older version though. Let me know if any more info is needed.

Node: 8.14.0
Os: win7

[ForbesLindesay]

This is caused by sync-rpc's attempts to do cleanup. If you attache the process.on('exit' handler before you require('sync-request') it works fine:

console.log('Process started');

process.on('exit', () => {
  try {
    let res = syncRequest('GET', 'https://jsonplaceholder.typicode.com/users/1');
    console.log(JSON.parse(res.getBody()));
  } catch(err) {
    console.log('Got error');
    console.log(err);
  }
});

const syncRequest = require('sync-request');

throw new Error('Boo');

If you can find a way to make this work regardless of ordering, I'd accept a PR to http://github.com/ForbesLindesay/sync-rpc. It would probably need to change https://github.com/ForbesLindesay/sync-rpc/blob/f500876453721b37376419ad855388aae0579ccf/lib/index.js#L38-L40 to give other process.on handlers a chance to run first.

[riddheshMarkandeya]

@ForbesLindesay, thanks for the response. Your solution works, also another way can be to put the exit event listener using process.prependListener('exit', ()=>{}), so it's called before the sync-rpc's exit listener. I guess this is simple enough so no fix is needed.

Also there is similar issue with SIGINT signal(replacing exit with SIGINT in the reproducer). In the 4.1.0 the child process never seem to receive the SIGINT(tested by putting SIGINT signal listener), but in the latest one the child process receives it and kills the request midway. Any workaround for that?
Tested on linux to make sure the separate nc process is created.

[riddheshMarkandeya]

Tested little bit more for SIGINT, for below script(hitting Ctrl+c when 'Process started' is printed):

const syncRequest = require('sync-request');  // v6.1.0
console.log('Process started');

process.on('SIGINT', () => {
  console.log('got SIGINT');
  try {
    let res = syncRequest('GET', 'https://jsonplaceholder.typicode.com/users/1');
    console.log(JSON.parse(res.getBody()));
  } catch(err) {
    console.log('Got error');
    console.log(err);
  }
});

setTimeout(() => {
  console.log('timeout boo');
}, 6000);

In windows above works till node version 8.11.4, and from 8.12.0 it stops working.
Interestingly on linux it's not working for any version.
So is it expected to not work?

Error is similar to exit:

got SIGINT
Got error
Error: nodeNC failed:

events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: connect ECONNREFUSED 127.0.0.1:50678
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1191:14)

    at sendMessage (C:\work\winston-test\node_modules\sync-rpc\lib\index.js:146:13)
    at C:\work\winston-test\node_modules\sync-rpc\lib\index.js:175:25
    at request (C:\work\winston-test\node_modules\sync-request\lib\index.js:28:15)
    at process.on (C:\work\winston-test\sync-request-test.js:75:15)
    at emitOne (events.js:116:13)
    at process.emit (events.js:211:7)
    at Signal.wrap.onsignal (internal/process.js:197:44)
timeout boo

[ForbesLindesay]

I think any kind of execution during exit is going to be a special case for sync-rpc. The problem is that one of the cleanup tasks we must do is killing the child process we spawn to handle the requests. There's no clean way I can think of for you to indicate that you have other cleanup tasks that must complete first.

[riddheshMarkandeya]

That makes sense, thanks @ForbesLindesay, feel free to close the issue.