Handling of ActiveRecord::RecordInvalid errors in actions

[manuelmeurer]

I discovered that if an action raises a ActiveRecord::RecordInvalid, the request returns a 422 response. In the UI, the view reloads, but there is no indication that an error happened.
If any other error is raised in an action, it is also raised in the UI (at least in development).

I assume this is not the desired behavior and ActiveRecord::RecordInvalid should be treated the same way as other errors when executing actions?

Trying to find the cause of this behavior, I thought is due to the fact that Avo::ActionsController inherits from Avo::BaseApplicationController, which includes the line

rescue_from ActiveRecord::RecordInvalid, with: :exception_logger

But when I delete that line, nothing changes.
I also couldn't find any spec that checks what happens when an error is raised in an action.
Where would be a good place to add such a spec?

[Paul-Bob]

Hi @manuelmeurer

Avo actions can be tied to a record or not. You might have an action that triggers a redirect request, or it can be tied to many records and models. The point is that actions do not handle ActiveRecord validation errors out of the box because of their vast range of possible use cases. However you can handle them manually. Depending on the nature of your action, something like this should work

class Avo::Actions::DeleteComment < Avo::BaseAction
  self.name = "Delete"

  def handle(query:, **args)
    errors = []
    query.each do |record|
      begin
        record.update!(body: nil)
      rescue ActiveRecord::RecordInvalid => error
        errors << "#{record.id}: #{error.message}"
      end
    end

    if errors.any?
      error "Failed due to:<br>#{errors.join("<br>")}"
    end
  end
end

In conclusion, I see this as a form of customization power rather than a limitation.

[manuelmeurer]

Yes, that works!
But I think it's a bit surprising that ActiveRecord::RecordInvalid is the only error that is caught and logged when it occurs in an action. What is the reasoning behind that?

[manuelmeurer]

Ah, you're right, I mixed those two up. 😢
That's why removing https://github.com/avo-hq/avo/blob/main/app/controllers/avo/base_application_controller.rb#L31 didn't change anything. 😅
Thanks for investigating!

[Paul-Bob]

I just found what's going on, and it's not about my first answer in this thread.

Rails rescues some errors and responds with different status codes by default. For example:

• ActiveRecord::RecordInvalid is catched and returns a 422 Unprocessable Content status code by default.
• ActiveRecord::RecordNotFound is catched and returns a 404 Not Found status code by default.

You can remove this behavior with an initializer, but that will change it across your entire application. Please make sure you research and confirm that this is what you really want, removing the default responses for these errors to get 500 codes instead:

# config/initializers/rescue_responses.rb

ActionDispatch::ExceptionWrapper.rescue_responses.delete('ActiveRecord::RecordNotFound')
ActionDispatch::ExceptionWrapper.rescue_responses.delete('ActiveRecord::RecordInvalid')

I think it's also possible with config.action_dispatch.show_exceptions, but I couldn't make it work in my environment.

---

Relevant references:

• https://stackoverflow.com/questions/27925282/activerecordrecordnotfound-raises-404-instead-of-500
• https://guides.rubyonrails.org/configuring.html#config-action-dispatch-show-exceptions

[manuelmeurer]

Wow, thanks a lot for finding that, @Paul-Bob! 👏
The SO answer is 10 years old, but it seems the behavior is still mainly the same. 😀

The question is whether this behavior makes sense in the context of actions.
IMO it doesn't, I want all errors that occur in actions to be raised, so I solved it like this in my apps (for now):
manuelmeurer/baseline@2c217bd
This defines a custom Error class for each action and wraps the handle call so that a MyAction::Error is raised every time an error occurs in an action. The actual error is then the cause of the MyAction::Error object.

I have been using this "custom error class and prepended error handler" setup for my service objects for some years, and it's been very solid. 😀
Conceptually, I like that I can call MyService and know that if an error is raised, it will be MyService::Error (and I can then look at the cause of that error to find out what really happened).

What do you think should be the default behavior in Avo?
If it should stay as it is, I think we should at least add a note in the action docs about this.

[Paul-Bob]

What do you think should be the default behavior in Avo?

Since the action handling code is all custom, and the errors come from that code, I think all errors should be raised and visible. We are on the same page, we could apply part of the code you shared on the handle method and make sure all errors are properly shown in development.

In production, I think we should log the error and return a generic failure notification message like "Something went wrong" instead of breaking the page. That way, the user can report "I used that action and got an error that something went wrong," and the developer can check the prod logs. In development, the error would still be visible so it can be reproduced and debugged directly.

[Paul-Bob]

Wow, thanks a lot for finding that, @Paul-Bob! 👏
The SO answer is 10 years old, but it seems the behavior is still mainly the same. 😀

No problem, I find it interesting

---

Thanks for sharing your approach and code!