Skip to content
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

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

Open
jpastoor opened this issue Aug 18, 2016 · 3 comments
Open
Labels

Comments

@jpastoor
Copy link
Collaborator

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()
@aik099
Copy link
Member

aik099 commented Aug 18, 2016

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.

@jpastoor
Copy link
Collaborator Author

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.

@aik099
Copy link
Member

aik099 commented Aug 18, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants