jira-api-restclient icon indicating copy to clipboard operation
jira-api-restclient copied to clipboard

Methods with local caches will not play nice with api response of false

Open jpastoor opened this issue 8 years ago • 3 comments

Since these methods iterate on the result before checking the value they might be getting a false returned from the Api and get errors in the foreach.

Affected (at least)

  • getFields()
  • getPriorities()
  • getStatuses()
  • getResolutions()

jpastoor avatar Aug 18 '16 13:08 jpastoor

I think that in such cases the ClientInterface implementation will get back some kind of universal HTTP error code, that we can use to throw exception. We do handle some cases already (see https://github.com/chobie/jira-api-restclient/blob/master/src/Jira/Api/Client/CurlClient.php#L138-L153), but apparently combination of response and HTTP code used in your case isn't handled.

aik099 avatar Aug 18 '16 13:08 aik099

Agree if you are suggesting that we might want to refactor the code to throw an exception in case of failure, and remove the boolean "false" response case altogether from the code.

jpastoor avatar Aug 18 '16 14:08 jpastoor

Agree if you are suggesting that we might want to refactor the code to throw an exception in case of failure, and remove the boolean "false" response case altogether from the code.

It's not the false in general (who knows, maybe JIRA will return json-encoded false and this is valid response). It's returning false when we should have thrown exception instead.

I'm afraid we need to analyze each API call defined in REST API (even if we don't use it) to see what needs to be done on our side.

For example HTTP 404 code surely means item not found and should end up with exception, but not treated as such. For that 404 code I've created separate task already.

aik099 avatar Aug 18 '16 14:08 aik099