diff --git a/README.md b/README.md index 2b1bb48..7bb9367 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A “bring-your-own-Node” linter for newer versions of ESLint: v8 and above. ## Installation ```ShellSession -apm install linter-eslint-node +ppm install AtomLinter/linter-eslint-node ``` The `linter` package will be installed for you if it’s not already present in your Atom installation. If you’re using an alternative `linter-*` consumer, the `linter` package can be disabled. diff --git a/lib/console.js b/lib/console.js index 39c5930..1de7d78 100644 --- a/lib/console.js +++ b/lib/console.js @@ -2,7 +2,16 @@ const CONSOLE = {}; -const isEnabled = atom.inDevMode() && !process.env.SILENCE_LOG; +function getEnabled () { + if (process.env.SILENCE_LOG) { return false; } + return atom.config.get('linter-eslint-node.advanced.enableLogging'); +} + +let isEnabled = getEnabled(); + +atom.config.observe('linter-eslint-node.advanced.enableLogging', () => { + isEnabled = getEnabled(); +}); function makeConsoleMethod (name) { return (...args) => { diff --git a/lib/job-manager.js b/lib/job-manager.js index 637db4d..ca6ecfc 100644 --- a/lib/job-manager.js +++ b/lib/job-manager.js @@ -9,6 +9,9 @@ import console from './console'; import Config from './config'; import ndjson from 'ndjson'; +// How log we wait for a job to finish before giving up. +const JOB_DURATION_TIMEOUT_MS = 15000; + function generateKey () { return randomBytes(5).toString('hex'); } @@ -28,6 +31,29 @@ class InvalidWorkerError extends Error { } } +class TimeoutError extends Error { + constructor (duration) { + super(`Operation timed out after ${duration}ms`); + this.name = 'TimeoutError'; + this.duration = duration; + } +} + +function rejectAfterDuration (duration) { + return new Promise((resolve, reject) => { + setTimeout(() => { + reject(new TimeoutError(duration)); + }, duration); + }); +} + +function timeout (promise) { + return Promise.race([ + rejectAfterDuration(JOB_DURATION_TIMEOUT_MS), + promise + ]); +} + // A class that handles creating, maintaining, and communicating with the // worker that we spawn to perform linting. class JobManager { @@ -83,7 +109,19 @@ class JobManager { this.worker = spawn(nodeBin, [this.workerPath]); // Reject this promise if the worker fails to spawn. - this.worker.on('error', reject); + this.worker.on('error', (...args) => { + console.warn('Worker exited with error:', ...args); + reject(args); + }); + + this.worker.on('exit', ({ code }) => { + if (code !== 0) { + // The worker exited unexpectedly. We don't need to restart it now; + // we can wait until there's another task for the worker, and we + // don't want to cause a restart loop. + console.warn('Worker exited with code:', code); + } + }); this.worker.stdout .pipe(ndjson.parse({ strict: false })) @@ -130,6 +168,7 @@ class JobManager { } killWorker (worker) { + worker ??= this.worker; if (!worker || worker.exitCode) { return; } worker.removeAllListeners(); worker.kill(); @@ -191,11 +230,13 @@ class JobManager { bundle.key = key; console.debug('JobManager#send:', bundle); - return new Promise((resolve, reject) => { - this.handlersForJobs.set(key, [resolve, reject]); - let str = JSON.stringify(bundle); - this.worker.stdin.write(`${str}\n`); - }); + return timeout( + new Promise((resolve, reject) => { + this.handlersForJobs.set(key, [resolve, reject]); + let str = JSON.stringify(bundle); + this.worker.stdin.write(`${str}\n`); + }) + ); } } diff --git a/lib/main.js b/lib/main.js index 64be46b..8d25bbb 100644 --- a/lib/main.js +++ b/lib/main.js @@ -241,9 +241,26 @@ export default { this.notified.invalidNodeBin = true; }, + notifyAboutTimeout (timeoutError, editor = null) { + let forASpecificEditor = ''; + if (editor && editor.getPath) { + forASpecificEditor = `for \`${editor.getPath()}\``; + } + atom.notifications.addError( + `linter-eslint-node: Job timed out`, { + description: `A linting job ${forASpecificEditor}timed out after ${timeoutError.duration} milliseconds.`, + dismissable: true + } + ); + }, + clearESLintCache () { console.debug('Telling the worker to clear its cache'); - this.jobManager.send({ type: 'clear-cache' }); + try { + this.jobManager.send({ type: 'clear-cache' }); + } catch (err) { + this.handleError(err, 'clear-cache'); + } }, // Show a bunch of stuff to the user that can help them figure out why the @@ -435,6 +452,13 @@ export default { }, handleError (err, type, editor = null) { + if (err.name && err.name === 'TimeoutError') { + // An operation timed out. We should tell the user but should not sleep + // the worker, since we have no idea what caused the timeout or if it's + // likely to recur. + this.notifyAboutTimeout(err, editor && editor.getPath()); + return null; + } if (err.name && err.name === 'InvalidWorkerError') { // The worker script never got created. Aside from notifying the user // (which will be skipped if they've already gotten such a message in @@ -519,7 +543,7 @@ export default { { text: 'Install linter-eslint', onDidClick () { - shell.openExternal(`https://atom.io/packages/linter-eslint`); + shell.openExternal(`https://web.pulsar-edit.dev/packages/linter-eslint`); } } ] diff --git a/lib/worker.js b/lib/worker.js index 18c7a82..916856d 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -409,13 +409,27 @@ async function processMessage (bundle) { if (require.main === module) { process.stdin.pipe(ndjson.parse({ strict: false })).on('data', processMessage); process.stdin.resume(); + process.on('uncaughtException', (error) => { error.uncaught = true; error.error = 'Unknown error'; - emitError( - JSON.stringify(error, Object.getOwnPropertyNames(error)) - ); + + // We're catching this exception so that we can try to emit it as JSON + // before exiting. That way `JobManager` can at least attempt to report + // something meaningful to the user. But emitting the error as JSON might + // _itself_ fail, so we've got to guard against that, lest we start an + // infinite loop. + try { + emitError( + JSON.stringify(error, Object.getOwnPropertyNames(error)) + ); + } finally { + process.exit(1); + } }); + process.title = `node (linter-eslint-node worker ${process.pid})`; + + // Signal to the package that we're ready to lint. emit({ type: 'ready' }); } diff --git a/package.json b/package.json index 523f263..bcd83d5 100644 --- a/package.json +++ b/package.json @@ -145,6 +145,13 @@ "type": "boolean", "default": true, "order": 3 + }, + "enableLogging": { + "title": "Enable Logging", + "description": "Log diagnostic messages to the developer console. If you want to file a bug against `linter-eslint-node`, these messages may be useful.", + "type": "boolean", + "default": false, + "order": 4 } } } diff --git a/spec/linter-eslint-node-spec.js b/spec/linter-eslint-node-spec.js index 5ae6c5b..4859c8a 100644 --- a/spec/linter-eslint-node-spec.js +++ b/spec/linter-eslint-node-spec.js @@ -667,4 +667,20 @@ describe('The eslint provider for Linter', () => { }); }); + describe("When JobManager#killWorker is called", () => { + it('kills the worker', async () => { + let { jobManager } = linterEslintNode; + expect(!!jobManager).toBe(true); + + let editor = await atom.workspace.open(paths.bad); + atom.project.setPaths([projectDir]); + + // Kick off a job, then immediately kill the worker. + lint(editor); + expect(!!jobManager.worker).toBe(true); + jobManager.killWorker(); + expect(jobManager.worker.killed).toBe(true); + }); + }); + });