alexbrazier / simple-update-notifier

Simple update notifier to check for npm updates for cli applications
MIT License
17 stars 9 forks source link

Running simple-update-notifier when offline takes 10s #25

Closed nicojs closed 3 months ago

nicojs commented 5 months ago

Running simple-update-notifier when offline takes (longer than) 10s when awaited. See this example:

const pkg = require(resolveFromRoot('package.json'));
await updateNotifier({ pkg, alwaysRun: true })
$ time node bin/my-app --help
[...]

real    0m10.388s
user    0m0.573s
sys     0m0.040s

Could we introduce some kind of network check so we don't try the notification when offline?

alexbrazier commented 5 months ago

I could try setting a timeout or adding a timeout option, but from my testing without internet it looks node seems to immediately return saying it can't connect

require('https')
    .get('https://registry.npmjs.org', res => console.log('hello'))
    .on('error', err => console.log('error'))

> error

I'm guessing if you're connected to a network that doesn't provide internet access it may attempt to connect and the 10s timeout may be coming from your network? Let me know if you have any thoughts?

nicojs commented 3 months ago

I've experimented with this a bit. It seems impossible to handle this correctly, as there is no easy way to 'know' whether you are online or not. When you then await the result, it might take 10 seconds to time out.

In the examples in the readme, the await wasn't present. I thought it was a mistake not to await promises, but it is probably a desired effect in this case.