coreybutler / node-windows

Windows support for Node.JS scripts (daemons, eventlog, UAC, etc).
Other
2.81k stars 356 forks source link

maxRestarts cannot be set to zero #233

Closed dsbert closed 5 years ago

dsbert commented 5 years ago

The truthy check prevents setting maxRestarts to zero.

https://github.com/coreybutler/node-windows/blob/bf0cf090f3bbcb1f56d8e1672d764b15aadd30f9/lib/daemon.js#L194-L198

dustingraham commented 5 years ago

Would abortOnError: true, suffice? What's the purpose of setting it to zero?

dsbert commented 5 years ago

@dustingraham

The following can be found in the source documentation.

    /**
     * @cfg {Number} [maxRestarts=3]
     * The maximum number of restarts within a 60 second period before haulting the process.
     * This cannot be _disabled_, but it can be rendered ineffective by setting a value of `0`.
     */

Does the documentation specify how this setting is used (or not used) if abortOnError is set to true? Right now, it will always be sent as a command line argument to the .exe

coreybutler commented 5 years ago

I think this is a bug. Line 198 should really be

value: config.hasOwnProperty('maxRestarts') ? config.maxRestarts : 3