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

Runner middleware does not handle file-list rejections #3396

Closed
nicojs opened this issue Nov 11, 2019 · 3 comments · Fixed by #3400 or karronoli/redpen#10 · May be fixed by Omrisnyk/npm-lockfiles#136, Omrisnyk/npm-lockfiles#137 or Omrisnyk/npm-lockfiles#139
Labels

Comments

@nicojs
Copy link
Contributor

nicojs commented Nov 11, 2019

Expected behaviour

When using karma with autoWatch: false and singleRun: false, you can use karma run to initiate runs. However, when a preprocessor rejects with an error, I would expect the run to complete with an error.

Actual behaviour

Karma server closes with an 11 11 2019 07:29:10.133:ERROR [karma-server]: UnhandledRejection (at least: on node 10 and up)

This is because there is no error handling in the runner middleware:
https://github.com/karma-runner/karma/blob/master/lib/middleware/runner.js#L88-L94

Environment Details

  • Karma version (output of karma --version): 4.4.1
  • Relevant part of your karma.config.js file: n/a

Steps to reproduce the behaviour

  1. mkdir tmp && cd tmp
  2. npm init --yes
  3. npm i karma karma-jasmine karma-chrome-launcher
  4. Create the following files:
    // karma.conf.js
    module.exports = (config) => config.set({
        browsers: ["Chrome"],
        files: ["./index.js"],
        frameworks: ['jasmine'],
        plugins: ['karma-*', require.resolve('./preprocessor')],
        preprocessors: {
            './index.js': ['always-error']
        },
        singleRun: false,
        watch: false
    });
    
    // index.js
    describe('test', () => {
        it('works', () => {});
    });
    
    // always-error-preprocessor.js
    let i = 0;
    function createPreprocessor() {
        return function (content, file, done) {
            console.log(`Processing ${file}`);
            i++;
            if (i < 2) {
                return done(undefined, content)
            } else {
                return done(new Error('Expected error!'));
            }
        }
    }
    module.exports = {
        'preprocessor:always-error': ['factory', createPreprocessor],
    }
  5. Open 2 console windows. In the first window, use karma start. Whenever it is finished with the test, run karma run in the second window.

Output of the first window:

$ npx  karma start
Processing C:/z/github/tmp/index.js
11 11 2019 07:41:22.100:WARN [karma]: No captured browser, open http://localhost:9876/
11 11 2019 07:41:22.139:INFO [karma-server]: Karma v4.4.1 server started at http://0.0.0.0:9876/
11 11 2019 07:41:22.141:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
11 11 2019 07:41:22.151:INFO [launcher]: Starting browser Chrome
11 11 2019 07:41:23.468:INFO [Chrome 78.0.3904 (Windows 10.0.0)]: Connected on socket HszEjx5TwDi8drcfAAAA with id 94172194
Chrome 78.0.3904 (Windows 10.0.0): Executed 1 of 1 SUCCESS (0.149 secs / 0.001 secs)
TOTAL: 1 SUCCESS
Processing C:/z/github/tmp/index.js
11 11 2019 07:41:31.194:ERROR [karma-server]: UnhandledRejection

Output of the second window:

$ npx karma run
C:\z\github\tmp\node_modules\karma\lib\runner.js:70
      throw e
      ^

Error: read ECONNRESET
    at TCP.onStreamRead (internal/stream_base_commons.js:111:27)
@nicojs
Copy link
Contributor Author

nicojs commented Nov 12, 2019

I'm implementing a fix for this. It will report the error as run result with an error message (which will include the preprocessor error text).

I think that this is what a user would expect, in line with Karma's design. If anyone disagrees, please let me know. Thanks.

@johnjbarton
Copy link
Contributor

+1 Unhandledrejections should always be an error.

johnjbarton pushed a commit that referenced this issue Nov 27, 2019
Add error handling for rejections on the file list methods in the `runner` middleware. As discussed in #3396 it does this by handling an error the same way an error in a run is handled.

Fixes #3396
karmarunnerbot pushed a commit that referenced this issue Apr 9, 2020
# [5.0.0](v4.4.1...v5.0.0) (2020-04-09)

### Bug Fixes

* install semantic-release as a regular dev dependency ([#3455](#3455)) ([1eaf35e](1eaf35e))
* **ci:** echo travis env that gates release after_success ([#3446](#3446)) ([b8b2ed8](b8b2ed8))
* **ci:** poll every 10s to avoid rate limit. ([#3388](#3388)) ([91e7e00](91e7e00))
* **middleware/runner:** handle file list rejections ([#3400](#3400)) ([80febfb](80febfb)), closes [#3396](#3396) [#3396](#3396)
* **server:** cleanup import of the removed method ([#3439](#3439)) ([cb1bcbf](cb1bcbf))
* **server:** createPreprocessor was removed ([#3435](#3435)) ([5c334f5](5c334f5))
* **server:** detection new MS Edge Chromium ([#3440](#3440)) ([7166ce2](7166ce2))
* **server:** replace optimist on yargs lib ([#3451](#3451)) ([ec1e69a](ec1e69a)), closes [#2473](#2473)
* **server:** Report original error message ([#3415](#3415)) ([79ee331](79ee331)), closes [#3414](#3414)

### Code Refactoring

* use native Promise instead of Bluebird ([#3436](#3436)) ([33a069f](33a069f)), closes [/github.com//pull/3060#discussion_r284797390](https://github.com//github.com/karma-runner/karma/pull/3060/issues/discussion_r284797390)

### Continuous Integration

* drop node 8, adopt node 12 ([#3430](#3430)) ([a673aa8](a673aa8))

### Features

* **docs:** document `DEFAULT_LISTEN_ADDR` constant ([#3443](#3443)) ([057d527](057d527)), closes [#2479](#2479)
* **karma-server:** added log to the server.js for uncaught exception ([#3399](#3399)) ([adc6a66](adc6a66))
* **preprocessor:** obey Pattern.isBinary when set ([#3422](#3422)) ([708ae13](708ae13)), closes [#3405](#3405)

### BREAKING CHANGES

* Karma plugins which rely on the fact that Karma uses Bluebird promises may break as Bluebird-specific API is no longer available on Promises returned by the Karma core
* **server:** Deprecated createPreprocessor removed, karma-browserify < 7 version doesn't work
* no more testing on node 8.
@karmarunnerbot
Copy link
Member

🎉 This issue has been resolved in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

anthony-redFox pushed a commit to anthony-redFox/karma that referenced this issue May 16, 2023
# [5.0.0](karma-runner/karma@v4.4.1...v5.0.0) (2020-04-09)

### Bug Fixes

* install semantic-release as a regular dev dependency ([karma-runner#3455](karma-runner#3455)) ([1eaf35e](karma-runner@1eaf35e))
* **ci:** echo travis env that gates release after_success ([karma-runner#3446](karma-runner#3446)) ([b8b2ed8](karma-runner@b8b2ed8))
* **ci:** poll every 10s to avoid rate limit. ([karma-runner#3388](karma-runner#3388)) ([91e7e00](karma-runner@91e7e00))
* **middleware/runner:** handle file list rejections ([karma-runner#3400](karma-runner#3400)) ([80febfb](karma-runner@80febfb)), closes [karma-runner#3396](karma-runner#3396) [karma-runner#3396](karma-runner#3396)
* **server:** cleanup import of the removed method ([karma-runner#3439](karma-runner#3439)) ([cb1bcbf](karma-runner@cb1bcbf))
* **server:** createPreprocessor was removed ([karma-runner#3435](karma-runner#3435)) ([5c334f5](karma-runner@5c334f5))
* **server:** detection new MS Edge Chromium ([karma-runner#3440](karma-runner#3440)) ([7166ce2](karma-runner@7166ce2))
* **server:** replace optimist on yargs lib ([karma-runner#3451](karma-runner#3451)) ([ec1e69a](karma-runner@ec1e69a)), closes [karma-runner#2473](karma-runner#2473)
* **server:** Report original error message ([karma-runner#3415](karma-runner#3415)) ([79ee331](karma-runner@79ee331)), closes [karma-runner#3414](karma-runner#3414)

### Code Refactoring

* use native Promise instead of Bluebird ([karma-runner#3436](karma-runner#3436)) ([33a069f](karma-runner@33a069f)), closes [/github.com/karma-runner/pull/3060#discussion_r284797390](https://github.com//github.com/karma-runner/karma/pull/3060/issues/discussion_r284797390)

### Continuous Integration

* drop node 8, adopt node 12 ([karma-runner#3430](karma-runner#3430)) ([a673aa8](karma-runner@a673aa8))

### Features

* **docs:** document `DEFAULT_LISTEN_ADDR` constant ([karma-runner#3443](karma-runner#3443)) ([057d527](karma-runner@057d527)), closes [karma-runner#2479](karma-runner#2479)
* **karma-server:** added log to the server.js for uncaught exception ([karma-runner#3399](karma-runner#3399)) ([adc6a66](karma-runner@adc6a66))
* **preprocessor:** obey Pattern.isBinary when set ([karma-runner#3422](karma-runner#3422)) ([708ae13](karma-runner@708ae13)), closes [karma-runner#3405](karma-runner#3405)

### BREAKING CHANGES

* Karma plugins which rely on the fact that Karma uses Bluebird promises may break as Bluebird-specific API is no longer available on Promises returned by the Karma core
* **server:** Deprecated createPreprocessor removed, karma-browserify < 7 version doesn't work
* no more testing on node 8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment