Meaningful error message on malformed registry response #1356
Conversation
| @@ -24,6 +24,10 @@ export default class NpmResolver extends RegistryResolver { | |||
| static registry = 'npm'; | |||
|
|
|||
| static async findVersionInRegistryResponse(config: Config, range: string, body: RegistryResponse): Promise<Manifest> { | |||
| if (!body['dist-tags']) { | |||
| throw new MessageError(`Received malformed response from registry. The registry may be down.`); | |||
There was a problem hiding this comment.
This needs to use this.reporter.lang, look for other uses in the codebase to see what I mean
| @@ -24,6 +24,10 @@ export default class NpmResolver extends RegistryResolver { | |||
| static registry = 'npm'; | |||
|
|
|||
| static async findVersionInRegistryResponse(config: Config, range: string, body: RegistryResponse): Promise<Manifest> { | |||
| if (!body['dist-tags']) { | |||
There was a problem hiding this comment.
this check feels very loose. I wonder if we can do better or if this is the best we can do...
There was a problem hiding this comment.
I was thinking the same thing, but I believe this is possibly the best indicator of a well-formed response, given what we want. Definitely open to suggestions, but I was specifically trying to avoid an error slightly lower down where body['dist-tags'] is undefined.
| @@ -24,6 +24,10 @@ export default class NpmResolver extends RegistryResolver { | |||
| static registry = 'npm'; | |||
|
|
|||
| static async findVersionInRegistryResponse(config: Config, range: string, body: RegistryResponse): Promise<Manifest> { | |||
| if (!body['dist-tags']) { | |||
| throw new MessageError(config.reporter.lang('malformedRegistryResponse')); | |||
There was a problem hiding this comment.
@thejameskyle Looking better, hopefully? Thanks for the review =)
| @@ -1,5 +1,5 @@ | |||
| HTTP/1.1 200 OK | |||
| Date: Wed, 19 Oct 2016 13:30:48 GMT | |||
| Date: Mon, 24 Oct 2016 13:03:20 GMT | |||
There was a problem hiding this comment.
I think this should be in the .gitignore, but I'm not sure what the generalized entry would be.
|
Currently I'm facing the same issue. Is there any time frame when it will be part of a official release? Thanks ... and great work! |
Summary
When the yarn DNS failed to resolve today, a malformed response from the registry caused a cryptic error (see #1348) rather than a meaningful message.
Sorry for no test plan; this is a simple commit, but I wouldn't know how to reproduce it.
npm teststill works, so it doesn't break anything. =)