-
Notifications
You must be signed in to change notification settings - Fork 50
Issue #50: HTTP errors are ignored and result in empty responses with… #55
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package io.ipinfo.api.errors; | ||
|
||
/** | ||
* <p>This exception is raised when a client error is occurring (Http status code like 4xx).</p> | ||
* | ||
* <p>Note that HTTP status 403 (Forbidden) is now reported as a checked exception of type @see InvalidTokenException .</p> | ||
*/ | ||
public class ClientErrorException extends HttpErrorException { | ||
public ClientErrorException(int statusCode, String message) { | ||
super(statusCode, message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package io.ipinfo.api.errors; | ||
|
||
/** | ||
* This covers all non 403 and 429 Http error statuses. | ||
*/ | ||
public class HttpErrorException extends RuntimeException { | ||
private final int statusCode; | ||
|
||
public HttpErrorException(int statusCode, String message) { | ||
super(message); | ||
this.statusCode = statusCode; | ||
} | ||
|
||
public int getStatusCode() { | ||
return statusCode; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package io.ipinfo.api.errors; | ||
|
||
/** | ||
* <p>This exception is raised when the service returns a 403 (access denied).</p> | ||
* | ||
* <p>That likely indicates that the token is either missing, incorrect or expired. | ||
* It is a checked exception so, if you have multiple tokens (example during transition), you can fall back to another token.</p> | ||
*/ | ||
public class InvalidTokenException extends Exception { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that we've introduced a proper This helps simplify the design (no need to disambiguate everywhere with |
||
public InvalidTokenException() { | ||
super("The server did not accepted the provided token or no token was provided."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package io.ipinfo.api.errors; | ||
|
||
/** | ||
* This exception is raised when a server error is returned by IpInfo (Http status like 5xx) | ||
*/ | ||
public class ServerErrorException extends HttpErrorException { | ||
public ServerErrorException(int statusCode, String message) { | ||
super(statusCode, message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
package io.ipinfo.api.request; | ||
|
||
import com.google.gson.Gson; | ||
import io.ipinfo.api.errors.ErrorResponseException; | ||
import io.ipinfo.api.errors.RateLimitedException; | ||
import io.ipinfo.api.errors.*; | ||
import okhttp3.Credentials; | ||
import okhttp3.OkHttpClient; | ||
import okhttp3.Request; | ||
|
@@ -18,9 +17,9 @@ protected BaseRequest(OkHttpClient client, String token) { | |
this.token = token; | ||
} | ||
|
||
public abstract T handle() throws RateLimitedException; | ||
public abstract T handle() throws RateLimitedException, InvalidTokenException; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be simplified to |
||
|
||
public Response handleRequest(Request.Builder request) throws RateLimitedException { | ||
public Response handleRequest(Request.Builder request) throws RateLimitedException, InvalidTokenException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, this can be simplified |
||
request | ||
.addHeader("Authorization", Credentials.basic(token, "")) | ||
.addHeader("user-agent", "IPinfoClient/Java/3.0.0") | ||
|
@@ -41,6 +40,12 @@ public Response handleRequest(Request.Builder request) throws RateLimitedExcepti | |
|
||
if (response.code() == 429) { | ||
throw new RateLimitedException(); | ||
} else if ((response.code() == 403)) { | ||
throw new InvalidTokenException(); | ||
} else if ((response.code() >= 400) && (response.code() <= 499)) { | ||
throw new ClientErrorException(response.code(), "Http error " + response.code() + ". " + response.message()); | ||
} else if ((response.code() >= 500) && (response.code() <= 599)) { | ||
throw new ServerErrorException(response.code(), "Http error " + response.code() + ". " + response.message()); | ||
} | ||
|
||
return response; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this have
throws IllegalArgumentException
?