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

fix(middleware): throw script tag errors #3585

Closed
wants to merge 1 commit into from

Conversation

johnjbarton
Copy link
Contributor

Include failing path. This is important for module tags which otherwise
simply give 404 at the server.

Fixes #3572

@johnjbarton
Copy link
Contributor Author

@jehon please review

@karmarunnerbot
Copy link
Member

Build karma 392 completed (commit b280f298c8 by @johnjbarton)

@AppVeyorBot
Copy link

Build karma 2789 completed (commit b280f298c8 by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 391 completed (commit b280f298c8 by @johnjbarton)

@jehon
Copy link

jehon commented Dec 15, 2020

Yes, this is what I found out yesterday night too...
Please note that the error on console will be shown twice in the case of an error in a "classical" script:

With this change:

Uncaught ReferenceError: <error msg>
ReferenceError: <error msg>
   at failing-test.js:2:1

Instead of before:

ReferenceError: <error msg>
   at failing-test.js:2:1

@johnjbarton
Copy link
Contributor Author

Hmm. Reference Error is unexpected. I need to add a test that throws.

Include failing path. This is important for module tags which otherwise
simply give 404 at the server.

Add e2e test to verify the message.

Fixes karma-runner#3572
@johnjbarton
Copy link
Contributor Author

Added an e2e test.

@karmarunnerbot
Copy link
Member

Build karma 396 completed (commit 95aa986f97 by @johnjbarton)

@AppVeyorBot
Copy link

Build karma 2793 completed (commit 95aa986f97 by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 395 completed (commit 95aa986f97 by @johnjbarton)

@jehon
Copy link

jehon commented Dec 16, 2020

@johnjbarton sorry for not being precise... the "ReferenceError" is the internal error in the loaded script. It is the error that I was expecting. Could have been anonymised a bit more...
It is the fact that the same error (referenceerror) appear twice that may be a problem...

@johnjbarton
Copy link
Contributor Author

Actually the non-module path is a problem, because by default karma loads any unknown file as js. If the config does not match the filesystem, a script error results. Need to fix that up.

@johnjbarton
Copy link
Contributor Author

This was superceded by #3605

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.

Failing to load a (esm) module does not fail the test suite
4 participants