AtomLinter / linter-codeclimate

An Atom Linter plugin for the Code Climate CLI
http://github.com/codeclimate/codeclimate
MIT License
10 stars 5 forks source link

JS error on package when linting multiple open files concurrently #62

Closed cgalvarez closed 7 years ago

cgalvarez commented 7 years ago

Steps to reproduce (preferable to be developing this package to see the codeclimate commands executed in the console):

  1. Open multiple files to be linted (I open one css and one yaml, for example).
  2. Close Atom.
  3. Re-open Atom with atom -dto keep your previous context (the files of step 1 should be opened again).
  4. Open dev tools and check the console. You should something like this:
linter-registry.js [sm]:144 [Linter] Error running Code Climate DOMException: Failed to execute 'measure' on 'Performance': The mark 'linter-codeclimate: Analysis-start' does not exist.
    at Error (native)
    at endMeasure (/opt/cgalvarez/linter-codeclimate/lib/index.js:34:17)
    at Object.<anonymous> (/opt/cgalvarez/linter-codeclimate/lib/index.js:244:9)
    at next (<anonymous>)
    at step (/opt/cgalvarez/linter-codeclimate/lib/index.js:7:273)(anonymous function) @ linter-registry.js [sm]:144

Because of long execution times of codeclimate (that's the reason behind PR #61), this should fail quite often (the affected code is not enclosed inside an if (atom.inDevMode()) { block, so it is run on every lint).

Running the affected code only in dev mode will still fire the error (although only in dev mode).

Besides, the message itself is not very detailed: linter-codeclimate: Analysis took: 6739.615000000001, so I think the easiest and most simple solution woud be using a string customized for the linted file, logging something like linter-codeclimate: Analysis for style.css took: 6739.615000000001.

cgalvarez commented 7 years ago

I confirm that my proposed solution fixes the issue and works fine. I'll send another PR later.

cgalvarez commented 7 years ago

I've discovered a new race condition on this same issue.

Although I fix the measure mark using the relative path to the file under linting, if you save that file multiple consecutive times (think of a typo that you quickly fix), it's possible to launch consecutive codeclimate calls for the same file, and the mark is the same for all of them.

If that mark is removed when the first result is received, then all the other calls will fail when reaching their endMeasure() call, because all of them try to clear a mark that has been already removed.

I think the most performant way to do this would be killing the previous and pending codeclimate executions by checking if the mark it's already in use, since we are only interested in the results from the last one. That would avoid executing codeclimate multiple times in parallel.