Error message returned if the track is locked #871
Conversation
There was a problem hiding this comment.
Thank you so much @Jrank2013 !
Would you be able to write a test that reproduces the problem and shows the fix?
cmd/download.go
Outdated
| body, err := ioutil.ReadAll(res.Body) | ||
| res.Body = ioutil.NopCloser(bytes.NewReader(body)) | ||
|
|
||
| if err := json.Unmarshal(body, &d.payload); err != nil || res.StatusCode < 200 || res.StatusCode > 299 { |
There was a problem hiding this comment.
The decodedAPIError already unmarshals the error, right? Can we move all the error handling logic into that method?
There was a problem hiding this comment.
The response body can only be read once. If we unmarshal or decode the body before saving a copy we wouldn't have a way to decode the JSON into the error struct.
| @@ -185,7 +187,14 @@ func newDownload(flags *pflag.FlagSet, usrCfg *viper.Viper) (*download, error) { | |||
| } | |||
| defer res.Body.Close() | |||
|
|
|||
| if err := json.NewDecoder(res.Body).Decode(&d.payload); err != nil { | |||
| if res.StatusCode < 200 || res.StatusCode > 299 { | |||
| return nil, decodedAPIError(res) | |||
There was a problem hiding this comment.
Now we have the decodedAPIError twice, which seems like it shouldn't be necessary.
I'm sorry if I'm being dumb, here. I'll take some time after work to pull this down and play around with it to see if I understand it better.
There was a problem hiding this comment.
Ok, I played with this and I totally see what's going on.
Sorry for being slow!
There was a problem hiding this comment.
No problem, just glad I can help!
There was a problem hiding this comment.
I'm going to merge this because it totally fixes the error.
I'm also going to spend a little bit of time rethinking the decodedAPIError function, because I'm not really happy with it right now (but that's nothing to do with this PR, more about decisions made previously).
Thanks for your patience in walking me through the fix, @Jrank2013 !
This fixes #870.
If the user has not unlocked the track that they have requested to download, the cli will return an error.