search

npm / npm-registry-client

http://npm.im/npm-registry-client
ISC License
264 stars 108 forks source link

request error is string? #112

Open alanshaw opened 9 years ago

alanshaw commented 9 years ago

https://github.com/npm/npm-registry-client/blob/a8d3193832487fb2e6b5015e30d15fe1b15f48e2/lib/request.js#L212

The error param is a string...

This line has pretty much not changed since the code was imported but the code I've been using used to get an error object back with a "E404" code. So something changed somewhere! Perhaps the server used to respond with some data but now doesn't.

Shall we change it to respond with an error object with a code? Happy to submit a PR.

othiym23 commented 9 years ago

Good find! I had to do some poking through the git history, and the bug has been there ever since npm-registry-client was extracted from the npm code base. It just happens that there have been some changes made to the registry reasonably recently that make it more likely that the registry will return a 404 with no message body, thus triggering this bug. #113 fixes the issue and will get pushed out with the next version of npm-registry-client, probably this week.

alanshaw commented 9 years ago

Thanks.

Other errors from this module are augmented with a code, statusCode and pkgid (https://github.com/npm/npm-registry-client/blob/a8d3193832487fb2e6b5015e30d15fe1b15f48e2/lib/request.js#L251-L253). Should we do the same here for consistency?

othiym23 commented 9 years ago

Probably all of the logic related to initializing the error should be hoisted into a helper function and then used everywhere we're returning errors, as we do elsewhere in the npm code base. Would you do me a favor and file that as a comment on #113, so it's closer to the code?