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

[FEATURE] Allow satisfies to throw errors #418

Open
Tracked by #501
jharrilim opened this issue Jan 11, 2022 · 2 comments
Open
Tracked by #501

[FEATURE] Allow satisfies to throw errors #418

jharrilim opened this issue Jan 11, 2022 · 2 comments
Labels
Enhancement new feature or improvement semver:major backwards-incompatible breaking changes
Milestone

Comments

@jharrilim
Copy link

What / Why

It would be nice to have satisfies throw an error so that we may see that the range does not satisfy due to an invalid range as opposed to an invalid version. The range error that we'd like to see is here:

throw new TypeError(`Invalid SemVer Range: ${range}`)

When

  • When verifying engine versions during npm install
  • When adding a dependency to package.json during npm install

Where

  • npm/node-semver

How

Current Behavior

Create an empty project with the following package.json:

{
  "name": "foo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "engines": {
    "node": ">=6.5.0 !15"
  },
  "engineStrict": true
}

Run npm install and you get:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'foo@1.0.0',
npm WARN EBADENGINE   required: { node: '>=6.5.0 !15' },
npm WARN EBADENGINE   current: { node: 'v16.11.1', npm: '8.0.0' }
npm WARN EBADENGINE }

up to date, audited 1 package in 414ms

found 0 vulnerabilities

Expected Behavior

Create an empty project with the following package.json:

{
  "name": "foo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "engines": {
    "node": ">=6.5.0 !15"
  },
  "engineStrict": true
}

Run npm install and ideally get something like:

npm WARN EBADENGINE Invalid SemVer Range: >=6.5.0 !15

up to date, audited 1 package in 414ms

found 0 vulnerabilities

Who

  • n/a

References

  • n/a
@lukekarrys lukekarrys added semver:major backwards-incompatible breaking changes Enhancement new feature or improvement labels Apr 10, 2022
@lukekarrys
Copy link
Member

I'm not opposed to this but I do think this would be a breaking change.

@AlanSl
Copy link

AlanSl commented Jun 16, 2022

This would also be very useful for debugging when a bug or failure in semver is encountered, such as #381 or #354

Currently, many errors or failures within semver return a false negative with no clue that something has failed - unless a debugger with "break on caught errors" is active, they appear exactly like a completed validation that has successfully judged the version to be invalid.

If what actually happened was semver hit an error mid-validation due to some bundling issue or internal bug, it's important to be able to see that and treat it differently, rather than have an operation continue based on possibly incorrect information.

(we've just had exactly this happen, and it took a long time to figure out that there was a caught internal error in semver caused by a bundling issue)

@darcyclarke darcyclarke added this to the v8 milestone Oct 31, 2022
@darcyclarke darcyclarke mentioned this issue Oct 31, 2022
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement new feature or improvement semver:major backwards-incompatible breaking changes
Projects
None yet
Development

No branches or pull requests

4 participants