search

Ingenico-ePayments / connect-sdk-java

Ingenico Connect Java Server SDK
https://docs.connect.worldline-solutions.com/documentation/sdk/server/java/
Other
31 stars 23 forks source link

NullPointerException instead of GlobalCollectException on http 500 #14

Closed benbenw closed 6 years ago

benbenw commented 6 years ago

In order to improve our application resilience, we are generating errors when calling the connect plateform. The calls are done with the connect sdk but the endpoint is a fake one. This mock endpoint returns all kinds of errors (4xx, 5xx, timeout, bad json, etc..)

Please note that those tests were made to verify our application not the sdk.

But for one of our test, the sdk is return a NullPointerException which is not the expected behaviour. In this case our mock returns a http status 500 with an empty body. The NPE is coming from : https://github.com/Ingenico-ePayments/connect-sdk-java/blob/e79e1d887cbe58f6fa2c8323bbc1a6d5a9c129da/src/main/java/com/ingenico/connect/gateway/sdk/java/ApiResource.java#L106

as the errorObject is null. It raises a NPE instead of the IllegalArgumentException

After reading the documentation, we think that this error should throw a GlobalCollectException and not a NPE or IllegalArgumentException.

Extract from the connect documentation :

A GlobalCollectException if something went wrong on the Ingenico ePayments platform. The Ingenico ePayments platform was unable to process a message from a downstream partner/acquirer, or the service that you're trying to reach is temporary unavailable (HTTP status code 500, 502 or 503)

rob-spoor commented 6 years ago

Can you show us the response that you are receiving? Keep in mind that the SDK is built to support the possible responses from the REST API. If you are generating something different than it's not strange that the SDK cannot handle such responses.

benbenw commented 6 years ago

That's something we have in mind BUT in case of a 500 error (Internal server error) the sdk should probably be ready to receive any body payload. We're not asking for a specific dedicated exception, but for a sdk exception eg GlobalCollectException or ApiException instead of a NPE.

The response is an empty body with a 500 http status.

rob-spoor commented 6 years ago

We will add a fix, but keep in mind that this does not warrant a hotfix release. It will be part of the next scheduled release.

benbenw commented 6 years ago

Thanx We can wait. It's not a blocking problem.

rob-spoor commented 6 years ago

The fix will include returning the same type of exception as would be thrown if there is a response body, except without the error id and error list. That means a 400 will cause a ValidationException to be thrown, a 403 a AuthorizationException, a 500 a GlobalCollectException, etc.

rob-spoor commented 6 years ago

This has been fixed in version 5.19.0.