Introduce ActiveRecord:Errors registry for error translations

[edwinv]

Motivation / Background

In our codebase, we heavily lean on PostgreSQL stored procedures. These procedures can raise errors through the RAISE statement. We've added custom error codes to these errors so we can throw custom exceptions in our Ruby code. Up until now this has always been a monkey patch to the translate_exception method in the PostgreSQL adapter. The upgrade to Rails 7.1 broke our monkey patch (again), this motivated us to work on a better solution in ActiveRecord.

Detail

This PR introduces the ActiveRecord:Errors registry to manage translations of database specific exceptions to ActiveRecord exceptions. Initially I wanted to only introduce this for custom errors, but after seeing the result I figured we can also use this for all specific translations in the adapters. This cleans up a lot of code and makes it easier to understand the translation.

A translation for an error can be registered in the registry:

ActiveRecord::Errors.register("MY0001", MyCustomException, adapter: ActiveRecord::ConnectionAdapters::PostgreSQL)
ActiveRecord::Errors.register(/connection lost/, CustomConnectionLostException, adapter: ActiveRecord::ConnectionAdapters::PostgreSQL)
ActiveRecord::Errors.register((e)-> { e.kind_of?(Exception) }, MyCustomException, adapter: ActiveRecord::ConnectionAdapters::PostgreSQL)

The AbstractAdapter does a lookup in the registry to find the appropriate ActiveRecord exception. By supporting strings, Regexp and Procs, almost all translation cases in the specific adapters can be replaced.

Additional information

Changing error handling for all database adapters should not be taken lightly. I've checked the test coverage for all errors and found that all errors are covered with tests spread over different test cases.

Checklist

Before submitting the PR make sure the following are checked:

• [x] This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• [x] Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• [x] Tests are added or updated if you fix a bug or add a feature.
• [x] CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[simi]

Nice! I was looking for this already few times. :muscle:

[byroot]

So I'm positive on the feature, but not really a fan of the implementation.

I think providing a single callback that receive the original exception and is free to raise or not would be simpler and much more flexible.

But also with https://github.com/rails/rails/pull/50140, it's now super easy to substitute a standard adapter by subclass that redefines translate_exception. But I see how a more blessed API would be valuable.

[edwinv]

So I'm positive on the feature, but not really a fan of the implementation.

I think providing a single callback that receive the original exception and is free to raise or not would be simpler and much more flexible.

But also with #50140, it's now super easy to substitute a standard adapter by subclass that redefines translate_exception. But I see how a more blessed API would be valuable.

Subclassing the adapter and changing the translate_exception still has a risk with changing internal APIs. I think a blessed API is better to prevent issues with future upgrades. The callback route is indeed simpler and more flexible. My main issue with my current implementation is the first argument being a string, Proc or Regexp, that is a smell that we maybe need another solution.

It would be nice to customize the exceptions within a specific model, but I think this is difficult because the queries are not executed in this context. So we probably need a callback definition on library level:

config.active_record.exception_handler = (exception, message:, sql:, binds:) -> { raise MyCustomException if exception.message.match?/../ }

The translate_exception method will then first call this exception handler. Only when the exception handler does not raise anything, the internal exceptions will be raised.

Downsides:

• In this approach, the user needs quite some understanding of the internals of the translation because not only the exception is available as an argument, but also message, sql and binds. Which are only needed to initialize the exception in some cases, but are never used for checking the original exception.
• The current exception translation code can't be cleaned up. Which is fine because they are internal and the PR is less likely to break something.

@byroot What do you think of this direction?

[byroot]

What do you think of this direction?

That's pretty much what I had in mind yes.