search

zkat / make-fetch-happen

Get in loser, we're making requests!
Other
384 stars 27 forks source link

include pkg name in integrity error msg #50

Closed aheckmann closed 6 years ago

aheckmann commented 6 years ago

There are a handful of open integrity issues on npm, one of which is a feature request to expose the name of the package which failed the integrity check. This PR adds the name of the failed package to the error message when available. Including the name of the package in the error message allows us to know which module to republish if the package-lock integrity field is ok.

NPM: 5.5.1 & 5.6.0 NodeJS: 4 & 8

Old log snippet:

events.js:141
      throw er; // Unhandled 'error' event
      ^

Error: sha512-ylcamPJlcckW+rK0NwuIN2bJP8a2uMsJv0LcYgd1/reaKd8xpLad2UNTAW8S4MQFfPzBCEVZH1KSjABfOR5IeA== integrity checksum failed when using sha512: wanted sha512-ylcamPJlcckW+rK0NwuIN2bJP8a2uMsJv0LcYgd1/reaKd8xpLad2UNTAW8S4MQFfPzBCEVZH1KSjABfOR5IeA== but got sha512-dTc1sOyafhR0/yiqgDJoXEf9J3r6TBGHg99Y4NIHMKuFwLPSIAPpLrDGoJOor8pPWc+pp2WUDNVQsUgtPXCjcQ==. (3936 bytes)

New log snippet:

events.js:141
      throw er; // Unhandled 'error' event
      ^

Error: sha512-ylcamPJlcckW+rK0NwuIN2bJP8a2uMsJv0LcYgd1/reaKd8xpLad2UNTAW8S4MQFfPzBCEVZH1KSjABfOR5IeA== integrity checksum failed when using sha512: wanted sha512-ylcamPJlcckW+rK0NwuIN2bJP8a2uMsJv0LcYgd1/reaKd8xpLad2UNTAW8S4MQFfPzBCEVZH1KSjABfOR5IeA== but got sha512-dTc1sOyafhR0/yiqgDJoXEf9J3r6TBGHg99Y4NIHMKuFwLPSIAPpLrDGoJOor8pPWc+pp2WUDNVQsUgtPXCjcQ==. (3936 bytes) for https://registry.npmjs.org/repository/npm-all/the-pkg-name/-/the-pkg-name-1.3.1.tgz

See https://github.com/npm/npm/issues/17459

zkat commented 6 years ago

I do not believe this belongs over here. This is already being done in pacote, where information is more contextual and complete, so the messages can be better (and cover a wider range of error cases).

Thanks for the contribution, and this was definitely a serious UX issue, so I want to appreciate the time taken to put this patch together. I'm closing this, though, since it's already implemented elsewhere. Cheers!