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

ResourceApiBase#handle_error() duplicate when clause #18

Open
lifeBCE opened this issue Jun 17, 2014 · 5 comments
Open

ResourceApiBase#handle_error() duplicate when clause #18

lifeBCE opened this issue Jun 17, 2014 · 5 comments

Comments

@lifeBCE
Copy link

lifeBCE commented Jun 17, 2014

when RuntimeError, ArgumentError, Fog::Compute::VcloudDirector::TaskError
  #weird error format coming back from fog
  [BAD_REQUEST, error.message.split('message=>')[1]]
when RuntimeError, ArgumentError, Fog::Compute::VcloudDirector::BadRequest
  [BAD_REQUEST, error.to_s]

The above kills the intended functionality for RuntimeError and ArgumentError exceptions. The second when clause will never be reached (for these exception types) and appears to be the valid handler (for these exception types).

@lifeBCE lifeBCE changed the title handle_error() duplicate where clause handle_error() duplicate when clause Jun 18, 2014
@lifeBCE lifeBCE changed the title handle_error() duplicate when clause ResourceApiBase#handle_error() duplicate when clause Jun 18, 2014
@huxoll
Copy link
Contributor

huxoll commented Jun 21, 2014

Check with Phillip; I don't believe this fix is correct.

@lifeBCE
Copy link
Author

lifeBCE commented Jun 21, 2014

Sorry, I don't understand. What is not correct? To have the same condition satisfied in two different when clauses is bad logic and the second will never be reached. If the first when clause is desired, the second should be addressed.

@huxoll
Copy link
Contributor

huxoll commented Jun 21, 2014

Check with Phillip for the exact use case. We have Fog throwing an error on a successful request, so we must swallow the error to allow the UI to respond successfully.

@lifeBCE
Copy link
Author

lifeBCE commented Jun 21, 2014

There might be some confusion on what I addressed. Unless Fog is throwing a RuntimeError or ArgumentError then my change should not affect whatever issues is happening with Fog. What I attempted to show in the above snippet was the duplication of RuntimeError and ArgumentError error handling. Looks like a copy and paste issue.

I'll verify with Phillip the copy and paste error I suspect here but even if Fog is throwing some weird RuntimeError or ArgumentError, this is not the way to handle it because it traps ALL RuntimeErrors or ArgumentErrors raised in any resource API. The history of the code base shows the second when clause to be the long standing original and the first is the new addition (what I believe to be the issue).

Just to be clear, there are 2 when clauses each handling 3 different exception conditions. The first two conditions in each when clause are exactly the same. The third is different and deals with Fog.

@pjschmidt3
Copy link
Contributor

This should be OK to my knowledge.

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

Successfully merging a pull request may close this issue.

3 participants