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

CP-1392 Redirected errors during System().import to karma().error. #130

Merged
merged 1 commit into from
Feb 24, 2016
Merged

CP-1392 Redirected errors during System().import to karma().error. #130

merged 1 commit into from
Feb 24, 2016

Conversation

FrederikNJS
Copy link
Contributor

Fixes #128

The issue in #128 was that if the System.import() function failed, then all the browsers would disconnect with a "Potentially unhandled rejection". This is mostly annoying for SyntaxErrors.

By catching the errors from System.import, and redirecting the message to karma.error(), karma-jspm is improved in the following ways:

  • The error is instead printed nicely in the terminal, allowing the user to understand the error.
  • The the browsers stay alive, such that the user can simply fix the syntax error and save, triggering a new test run.
  • Not having to restart karma every time you make a syntax error.

Before a syntax error would result in the following output:

PhantomJS 2.1.1 (Linux 0.0.0) ERROR: 'Potentially unhandled rejection [3] eval@[native code]
__exec@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:1395:16
execute@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:3521:20
linkDynamicModule@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:3126:36
link@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:2969:28
execute@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:3301:17
doDynamicExecute@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:703:32
link@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:905:36
doLink@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:557:11
updateLinkSetOnLoad@http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:605:24
http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:417:30
tryCatchReject@http://localhost:9877/base/jspm_packages/system-polyfills.src.js?3aa57969dce4ecea4c51aab540f372d15cc886b6:1252:34
runContinuation1@http://localhost:9877/base/jspm_packages/system-polyfills.src.js?3aa57969dce4ecea4c51aab540f372d15cc886b6:1211:18
when@http://localhost:9877/base/jspm_packages/system-polyfills.src.js?3aa57969dce4ecea4c51aab540f372d15cc886b6:999:20
run@http://localhost:9877/base/jspm_packages/system-polyfills.src.js?3aa57969dce4ecea4c51aab540f372d15cc886b6:890:17
_drain@http://localhost:9877/base/jspm_packages/system-polyfills.src.js?3aa57969dce4ecea4c51aab540f372d15cc886b6:166:22
drain@http://localhost:9877/base/jspm_packages/system-polyfills.src.js?3aa57969dce4ecea4c51aab540f372d15cc886b6:131:15'

Chrome 48.0.2564 (Linux 0.0.0) ERROR: 'Potentially unhandled rejection [3] SyntaxError: missing ) after argument list
    Evaluating http://localhost:9877/base/test/brokentest.js
    Error loading http://localhost:9877/base/test/brokentest.js
    at eval (native)
    at SystemJSLoader.__exec (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:1395:16)
    at entry.execute (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:3521:16)
    at linkDynamicModule (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:3126:32)
    at link (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:2969:11)
    at Object.execute (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:3301:13)
    at doDynamicExecute (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:703:25)
    at link (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:905:20)
    at doLink (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:557:7)
    at updateLinkSetOnLoad (http://localhost:9877/base/jspm_packages/system.src.js?38538ebca96bc7b222f7e7ba15943f173a485f6e:605:18)'

19 02 2016 13:17:04.301:WARN [PhantomJS 2.1.1 (Linux 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.

19 02 2016 13:17:04.344:WARN [Chrome 48.0.2564 (Linux 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.

With this pull request, the output is instead:

PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  SyntaxError: Expected token ')'
    Evaluating /home/frederiknjs/Code/personal/karma/test/brokentest.js
    Error loading /home/frederiknjs/Code/personal/karma/test/brokentest.js

Chrome 48.0.2564 (Linux 0.0.0) ERROR
  SyntaxError: missing ) after argument list
    Evaluating /home/frederiknjs/Code/personal/karma/test/brokentest.js
    Error loading /home/frederiknjs/Code/personal/karma/test/brokentest.js

Hope you like it :-)

This is primarily useful for SyntaxErrors, as browsers no longer
shutdown when encountering a SyntaxError.

Signed-off-by: Frederik Nordahl Jul Sabroe <frederikns@gmail.com>
@maxwellpeterson-wf
Copy link
Contributor

+1 very nice!
@trentgrover-wf @evanweible-wf @jayudey-wf

@jayudey-wf jayudey-wf changed the title Redirected errors during System.import() to karma.error(). Redirected errors during System.import to karma.error. Feb 23, 2016
@jayudey-wf jayudey-wf changed the title Redirected errors during System.import to karma.error. CP-1392 Redirected errors during System.import to karma.error. Feb 23, 2016
@jayudey-wf jayudey-wf changed the title CP-1392 Redirected errors during System.import to karma.error. CP-1392 Redirected errors during System().import to karma().error. Feb 23, 2016
@evanweible-wf
Copy link
Contributor

Awesome! +1

@FrederikNJS
Copy link
Contributor Author

Interestingly, the output is even better in another of my projects. I'm not sure why, but it might just be because there is more code. Here's an example:

PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  SyntaxError: /home/frederiknjs/Code/personal/gw2map/javascript/model/region.js: Unexpected token (11:2)
     9 |     this._zones = regionDef.get('maps').valueSeq().map(mapDef=>new Zone(mapDef, iconUrls))
    10 |     this.name = regionDef.get('name').
  > 11 |   }
       |   ^
    12 | 
    13 |   get zones() {
    14 |     return this._zones.filter(zone => !this._Zone.falseZones.has(zone.id))
    Error loading /home/frederiknjs/Code/personal/gw2map/javascript/model/region.js as "javascript/model/region" from /home/frederiknjs/Code/personal/gw2map/test/model/floor-spec.js

@jayudey-wf
Copy link
Contributor

QA Resource Approval: +10

  • Testing instruction (derived from offline conversation with max
  • Dev +1's
  • Dev/QA +10 with detail of what was tested
    • installed into project that utilizes karma-jspm, verified that test task was still working as expected, also verified that more helpful messages were thrown during test and watch tasks
  • Unit test created/updated
  • All unit tests pass

Merging into master.

jayudey-wf added a commit that referenced this pull request Feb 24, 2016
CP-1392 Redirected errors during System().import to karma().error.
@jayudey-wf jayudey-wf merged commit e02e9b9 into computmaxer:master Feb 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browsers disconnect when encountering syntax error
4 participants