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.

Sign up for GitHub

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

Jump to bottom

syntax errors in package-scripts.js should not be swallowed. #22

Closed
jisaacks opened this issue Jun 8, 2016 · 5 comments · Fixed by #23
Closed

syntax errors in package-scripts.js should not be swallowed. #22

jisaacks opened this issue Jun 8, 2016 · 5 comments · Fixed by #23
Labels
enhancement help wanted

Comments

@jisaacks
Copy link
Collaborator

jisaacks commented Jun 8, 2016

  • p-s version: 1.0.1
  • node version: 6.2.1
  • npm version: 3.9.3

Problem description:

If there is a syntax error in the package-scripts.js (in my case for example a missing comma) then it will fail to load. However your error message swallows the syntax error and just says that it cannot find the package-scripts.js file which was confusing for me.

Suggested solution:

Basically when capturing the error here

Just add a check for syntax error:

if (e instanceof SyntaxError) {
   throw e;
}

I'll gladly make the change and submit a PR.
@kentcdodds
Copy link
Collaborator

Oh that's great! Yes, let's do that 👍 Thanks!

jisaacks added a commit to jisaacks/p-s that referenced this issue Jun 8, 2016
@jisaacks
Don't swallow SyntaxErrors when requiring module
f8fa583 
* Fixes sezna#22
* If the package-scripts.js file fails to load due to syntax error, don't swallow the error.
@jisaacks jisaacks mentioned this issue Jun 8, 2016
Merged
kentcdodds pushed a commit that referenced this issue Jun 8, 2016
@jisaacks
fix(bin-utils): Don't swallow syntax errors
367f274 
* fix(bin-utils): Don't swallow syntax errors

When trying to require the package-scripts.js if it failed due to a syntax error it would say it
could not find the file which was misleading.

* test(bin-utils): Add tests for not swallowing syntax errors

* docs(contributors): Added myself as a contributor

Closes #22
@kentcdodds
Copy link
Collaborator

I think this is a problem again...

@kentcdodds kentcdodds reopened this Feb 21, 2017
@boneskull
Copy link
Contributor

boneskull commented May 27, 2017 •
edited

Indeed, my errors are getting eaten.

UPDATE: SyntaxErrors are not being eaten, but any other exception your config file might throw (e.g., a typo in a require('something')) is swallowed.

@boneskull
Copy link
Contributor

I feel like the problem here is a lack of distinction between a missing config file, and one which is simply invalid.

@boneskull boneskull mentioned this issue May 27, 2017
Merged
kentcdodds pushed a commit that referenced this issue May 27, 2017
@boneskull
fix(cli): do not swallow any exceptions thrown by JS config files (#140)
e607e06 
A missing config file will report a warning, but any exception that the config file itself throws
will be reported to the user.  "Empty" configs are now also considered invalid; e.g. a 0-byte .js
file or one exporting an plain empty object.

#22
@Miklet
Copy link
Collaborator

Miklet commented Jul 25, 2017

Already merged, closing :)

@Miklet Miklet closed this as completed Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't swallow syntax errors in package-scripts.js jisaacks/p-s
4 participants
@boneskull @jisaacks @kentcdodds @Miklet