Skip to content

Conversation

@nphmuller
Copy link
Contributor

When the reponse body from the Spotify servers would be empty, there could be a NullReferenceException in the Web client. This, for example, did at least occur in every call to SaveAlbums.

@JohnnyCrazy
Copy link
Owner

JohnnyCrazy commented Jul 23, 2016

Thanks for catching this, I totally forgot about empty bodies when adding the Header-stuff
While this technically fixes the problem, it doesn't provide the headers (which should always be provided) if the body is empty.

Another approach would be adding a empty "{}" if the response is empty. This way, an ErrorResponse will be created and headers can be set.

I would exchange this line: SpotifyWebClient.cs#L121 with:

//Added empty object if data is empty --> Object will be created --> Headers can be retrieved
 response = new Tuple<ResponseInfo, string>(data.Item1, data.Item2.Length > 0 ? _encoding.GetString(data.Item2) : "{}"); 

Thoughts?

This fixes a possible NullReferenceException, which at least occurred in every call to SaveAlbums().
@nphmuller
Copy link
Contributor Author

Totally agree. It's much better to fix the root cause.

I also implemented the fix on the Download methods, since it is theoretically possible for the bug to also occur there.

@JohnnyCrazy
Copy link
Owner

JohnnyCrazy commented Jul 23, 2016

Perfect, we may consider adding a check to HasError() checking the status-header in the future, but since every error comes with an error-body, we should be good to go

@JohnnyCrazy JohnnyCrazy merged commit a5c584a into JohnnyCrazy:master Jul 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants