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

Don't respond with 400 on ArgumentError #174

Merged

Conversation

imtayadeway
Copy link
Contributor

If an ArgumentError was raised it means that we either did something
wrong or we did not handle the request properly and we should respond
approriately with a 500.

@miq-bot add-label bug
@miq-bot assign @abellotti

If an `ArgumentError` was raised it means that we either did something
wrong or we did not handle the request properly and we should respond
approriately with a 500.
@chrisarcand
Copy link
Member

chrisarcand commented Nov 2, 2017

From @mkanoor : "for AutomateWorkspaces if we got an ID instead of a GUID I wanted to raise an Argument Error"

We need to get something from automate to be able to respond accordingly for this (even if it's just... rescuing ArgumentError - from callers of automate)

@@ -1,6 +1,7 @@
module Api
class AutomateWorkspacesController < BaseController
def edit_resource(type, id, data = {})
raise BadRequestError, "must contain at least one attribute to edit" if data.blank?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkanoor is this the only case from the model that was raising the ArgumentError ?

@miq-bot
Copy link
Member

miq-bot commented Nov 2, 2017

Checked commit imtayadeway@5b6a866 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@mkanoor
Copy link
Contributor

mkanoor commented Nov 2, 2017

👍

@abellotti
Copy link
Member

one last question Gaprindashvili/yes ?

@imtayadeway
Copy link
Contributor Author

@abellotti sorry! Forgot to add. I consider this a bug, so adding Gaprindashvili/yes

@abellotti
Copy link
Member

Thanks @imtayadeway. 👍

@abellotti abellotti added this to the Sprint 73 Ending Nov 13, 2017 milestone Nov 2, 2017
@abellotti abellotti merged commit efe7610 into ManageIQ:master Nov 2, 2017
simaishi pushed a commit that referenced this pull request Nov 6, 2017
…quest

Don't respond with 400 on ArgumentError
(cherry picked from commit efe7610)
@simaishi
Copy link
Contributor

simaishi commented Nov 6, 2017

Gaprindashvili backport details:

$ git log -1
commit e797bad6d18fa35516dbd2a3d0947adb0fe2f5f0
Author: Alberto Bellotti <abellotti@users.noreply.github.com>
Date:   Thu Nov 2 17:47:49 2017 -0400

    Merge pull request #174 from imtayadeway/argument-error-isnt-a-bad-request
    
    Don't respond with 400 on ArgumentError
    (cherry picked from commit efe76104c2cc7542e997f54bde55798f1a1938c1)

@imtayadeway imtayadeway deleted the argument-error-isnt-a-bad-request branch January 12, 2018 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants