exceptionless / Exceptionless.JavaScript

Exceptionless JavaScript client
http://exceptionless.com
Apache License 2.0
59 stars 24 forks source link

QueueTimer keeps Node process from terminating gracefully #42

Closed frankebersoll closed 1 year ago

frankebersoll commented 8 years ago
var client = require('exceptionless').ExceptionlessClient.default;
client.config.apiKey = "***";
client.config.useDebugLogger();

This program will run and exit as expected. No events are submitted. Console output:

[info] Exceptionless: Processing queue...
franks-macbook:Exceptionless.JavaScript Frank$ 

Whereas, if we modify the code to submit an event:

var client = require('exceptionless').ExceptionlessClient.default;
client.config.apiKey = "***";
client.config.useDebugLogger();
client.submitLog('Test');

This program will run forever, unless you terminate it by sending SIGINT:

[info] Exceptionless: Enqueuing event: 1456470547394 type=log 
[info] Exceptionless: Processing queue...
[info] Exceptionless: Sending 1 events to https://collector.exceptionless.io.
[info] Exceptionless: Sent 1 events.
[info] Exceptionless: Finished processing queue.
[info] Exceptionless: Processing queue...
[info] Exceptionless: Processing queue...
[info] Exceptionless: Processing queue...

If we npm install wtfnode and call .dump(), it clearly shows what's going on:

[WTF Node?] open handles:
- Sockets:
  - undefined:undefined -> undefined:undefined
    - Listeners:
  - undefined:undefined -> undefined:undefined
    - Listeners:
- Timers:
  - (10000 ~ 10 s) wrapper @ /Users/Frank/Entw..../exceptionless.node.js:242

From Node docs:

In Node there is no such start-the-event-loop call. Node simply enters the event loop after executing the input script. Node exits the event loop when there are no more callbacks to perform.

So, our timer is the last single callback in Node's event loop and therefore blocks it from exiting. This is probably not an issue for service-like applications, but it sucks for console tools or anything that should just exit when the work is finished.

Thoughts:

niemyjski commented 8 years ago

Awesome find! There is no reason our queue timer can't turn itself off and then enqueue turns it back on, thoughts? We could use a timestamp to see when it should fire right away or in the future? Thoughts?

Maybe we could see what wtf node does and turn off? Or we could cancel the timer if no events have been submitted in the last 2 timer hits? The shorter wait the better, if my app that I just closed stays open longer than I expect thats frustrating. If there are network problems it should just bail (you should have local storage if you want those errors. No difference for different storage implementations.

niemyjski commented 8 years ago

@frankebersoll would you mind taking a stab at this.

srijken commented 8 years ago

Maybe this is why we need the process.exit() in the node tests? If work is done on this issue, we should also check if we can do without the process.exit() (See gulpfile)

niemyjski commented 8 years ago

Possibly, would be a good idea to do that with our tests for now. I'm not sure what the best way to handle this.

niemyjski commented 1 year ago

@srijken do you think this is something we should still tackle? I'm currently fine with the current behavior but I get there is an issue with cli tooling? I wonder what would happen for node lambdas.

niemyjski commented 1 year ago

This has been solved in version 2.0. I haven't figured out how to detect the last statement has been executed. But if you end your app with await Exceptionless.suspend(); it processes the queue and stops all the timers allowing the app to exit.

import { Exceptionless } from "@exceptionless/node";
console.log("Cli Example");
...
await Exceptionless.suspend();
ejsmith commented 1 year ago

I want us to try unref'ing the timer we are using and wiring up to the process exit event to try and flush the queue then.

niemyjski commented 1 year ago

await Exceptionless.suspend(); is no longer required.