Skip to content

Fix: Handle HTTP NO CONTENT status code (204) to prevent null reference exceptions in HttpBase.cs #188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 9, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/Proyecto26.RestClient/Helpers/HttpBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ namespace Proyecto26
{
public static class HttpBase
{
public static int HTTP_NO_CONTENT = 204;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be a const instead of static. Might be good to scope it private instead of public too, since it doesn't seem like anything outside of HttpBase will need to know about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@L-Naej hello mate, thanks for your contribution! Please check the @StevenGarberg's comment and let us know! 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me change that from develop branch, thanks for your help @StevenGarberg! <3

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're welcome! Thank you for making such a great library @jdnichollsc. I use it in all of my Unity projects to talk to my web APIs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StevenGarberg do you want to be a maintainer of this lib? <3

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdnichollsc Absolutely I do!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry guys, was completely off from internet for personal reasons, thank you very much for handling this!
Indeed I don't know what I was thinking with these public static, silly mistake...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha no problem, thanks for your help! <3


public static IEnumerator CreateRequestAndRetry(RequestHelper options, Action<RequestException, ResponseHelper> callback)
{

Expand Down Expand Up @@ -118,7 +120,7 @@ public static IEnumerator DefaultUnityWebRequest<TResponse>(RequestHelper option
var body = default(TResponse);
try
{
if (err == null && res.Data != null && options.ParseResponseBody)
if (err == null && res.StatusCode != HTTP_NO_CONTENT && res.Data != null && options.ParseResponseBody)
body = JsonUtility.FromJson<TResponse>(res.Text);
}
catch (Exception error)
Expand All @@ -139,7 +141,7 @@ public static IEnumerator DefaultUnityWebRequest<TResponse>(RequestHelper option
var body = default(TResponse[]);
try
{
if (err == null && res.Data != null && options.ParseResponseBody)
if (err == null && res.StatusCode != HTTP_NO_CONTENT && res.Data != null && options.ParseResponseBody)
body = JsonHelper.ArrayFromJson<TResponse>(res.Text);
}
catch (Exception error)
Expand Down