Skip to content

Match foreign key constraint error causes on exception names#6308

Merged
tvdeyen merged 2 commits intosolidusio:mainfrom
mamhoff:db-adapter-rescue-strings
Jul 26, 2025
Merged

Match foreign key constraint error causes on exception names#6308
tvdeyen merged 2 commits intosolidusio:mainfrom
mamhoff:db-adapter-rescue-strings

Conversation

@mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Jul 26, 2025

Summary

We've added pretty sophisticated error handling to the migrations that add foreign keys. When I first hit one of these errors, though, I found that in a Solidus context, only the error classes for the currently used database adapter will be loaded. Because our migrations need to work with all adapters, this PR changes the matching to be on the string name of the exception, rather than on the exception class.

Additionally, this changes the messaging to use ActiveRecord::Migration#say rather than directly invoking Rails.logger.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

mamhoff added 2 commits July 26, 2025 17:00
When migrating the database in a Solidus context, we usually only have
the database adapter loaded that is being used. Rescuing exceptions from
other adapters will therefore fail with an "uninitialized constant
error". In order to safeguard from this, this commit changes the
matching on error class to matching on their name.
The standard way of producing output for users to read in a migration is
`ActiveRecord::Migration#say`.
@mamhoff mamhoff requested a review from a team as a code owner July 26, 2025 15:07
@github-actions github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Jul 26, 2025
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks

@tvdeyen tvdeyen merged commit 424f284 into solidusio:main Jul 26, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:solidus_core Changes to the solidus_core gem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants