Skip to content

Share one KeyError path across rubies with and without detailed_message - #1836

Open
edgibbs wants to merge 1 commit into
thoughtbot:mainfrom
edgibbs:fix-trait-error-message-on-older-rubies
Open

Share one KeyError path across rubies with and without detailed_message#1836
edgibbs wants to merge 1 commit into
thoughtbot:mainfrom
edgibbs:fix-trait-error-message-on-older-rubies

Conversation

@edgibbs

@edgibbs edgibbs commented Sep 6, 2026

Copy link
Copy Markdown

Summary

👋 I ran into this while running bundle exec rake to get a baseline before
picking up an unrelated issue — the suite fails on a clean checkout of main.

main has been red since #1824 merged on 2026-08-21
(failing run).
Five jobs fail:

job failure
Ruby 3.1 / Rails 7.0, 7.1, 7.2 12 specs
Ruby jruby-9.4 / Rails 7.0 12 specs
Run standard 3 Layout/* offenses

#1824 builds the trait-not-found message separately in each of two
Ruby-version branches, and the two drifted apart — one emits "trait". and
Referenced within, the other "trait" and referenced within. The specs
assert a single format, so every ruby taking the second branch fails.

The remaining failure is traits_spec.rb:497: error_options, added by #1824
so did_you_mean can suggest a factory's own traits, was only wired into the
Ruby >= 3.2 branch. On older rubies there was no suggestion data at all.

Why CI didn't catch it

#1824 came from a fork, so its workflow runs needed maintainer approval and
never got it — the PR shows no checks at all. The failure only surfaced on the
post-merge push to main.

The fix

did_you_mean writes its suggestions into KeyError#message before Ruby 3.2 and
into #detailed_message after. Rebuilding the error with the factory's own
traits added to the candidates regenerates them on older rubies and returns the
message untouched on newer ones — so one implementation serves both, and the
KeyError.method_defined?(:detailed_message) conditionals in Definition and
Registry are both deleted rather than repaired. Net −3 lines.

Dropping Registry's conditional also means every registry miss now carries
key and receiver on older rubies, where they previously raised
ArgumentError. Messages are unchanged; factory and strategy lookups simply
match the behaviour they already had on Ruby >= 3.2.

Testing

No new specs. The 12 added by #1824 already cover this — they fail on Ruby 3.1
and jruby-9.4 before this change and pass after. Adding more would duplicate
them.

Verified with bundle exec rake all_specs on:

  • MRI 3.3.6 — 333 + 441 examples, 18 cucumber steps, standard clean
  • MRI 3.1.7 — 774 examples
  • JRuby 9.4.15.0 — 774 examples

…age`

Ruby-version branches, and the two drifted apart:

- one emits `"trait".` and `Referenced within`
- the other `"trait"` and `referenced within`

The specs assert a single format, so 12 fail on Ruby 3.1 and jruby-9.4,
and three Layout offenses from the same commit fail `standard`. `main`
has been red since 2026-08-21.

`did_you_mean` writes its suggestions into `KeyError#message` before Ruby
3.2 and into `#detailed_message` after. Rebuilding the error with the
factory's own traits added to the candidates regenerates them on older
rubies and returns the message untouched on newer ones, so one
implementation serves both and the version conditionals in `Definition`
and `Registry` both go away.

Dropping `Registry`'s conditional also means every registry miss now
carries `key` and `receiver` on older rubies, where they previously
raised `ArgumentError`.

The 12 specs added by thoughtbot#1824 already cover the message drift; they fail
before this change and pass after. Verified with `rake all_specs` on MRI
3.3.6, MRI 3.1.7 and JRuby 9.4.15.0.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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 this pull request may close these issues.

1 participant