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

QueueTimer keeps Node process from terminating gracefully #42

Closed
frankebersoll opened this issue Feb 26, 2016 · 8 comments
Closed

QueueTimer keeps Node process from terminating gracefully #42

frankebersoll opened this issue Feb 26, 2016 · 8 comments
Labels

Comments

@frankebersoll
Copy link
Contributor

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:

  • Can we use a shorter timespan for the timer in combination with a lastEventTimestamp field?
  • Can we leverage the same private API that wtfnode uses to figure out if we are the single reason that the process is still running?
  • How long do we want to wait for events to be submitted if the program has no work to do otherwise, before exiting? Should it wait forever if events cannot be submitted due to network problems? Should it make a difference between InMemory and LocalStorage?
@niemyjski
Copy link
Member

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 niemyjski added the bug label Sep 23, 2016
@niemyjski
Copy link
Member

@frankebersoll would you mind taking a stab at this.

@srijken
Copy link
Contributor

srijken commented Sep 27, 2016

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
Copy link
Member

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
Copy link
Member

@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
Copy link
Member

niemyjski commented Mar 1, 2023

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
Copy link
Member

ejsmith commented Mar 2, 2023

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
Copy link
Member

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

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

No branches or pull requests

4 participants