Skip to content

Fix #445: guard log line in roll against unset what/details#453

Merged
yegor256 merged 2 commits into
zerocracy:masterfrom
bibonix:fix-conclude-roll-unset-attrs
May 6, 2026
Merged

Fix #445: guard log line in roll against unset what/details#453
yegor256 merged 2 commits into
zerocracy:masterfrom
bibonix:fix-conclude-roll-unset-attrs

Conversation

@bibonix

@bibonix bibonix commented May 6, 2026

Copy link
Copy Markdown
Contributor

@yegor256 — this PR fixes #445 (Fbe::Conclude#roll crashing with RuntimeError: Can't find 'what' attribute when the draw block returns a non-String).

What changes

Per your direction in the issue ("the second option is better: roll should guard the log line against unset attributes"), the @loog.info("#{n.what}: #{n.details}") line in roll (lib/fbe/conclude.rb) now first checks n.all_properties and only emits the log line when both what and details are present on the new fact. The behaviour of fill is unchanged — it still only sets what/details when the block returns a String — so existing well-behaved judges keep their info-level log entries.

Why this approach

Factbase::Fact raises RuntimeError (not NoMethodError) when reading an unset property, so a generic rescue would be too broad. Using the public all_properties accessor (already used elsewhere in the codebase, e.g. lib/fbe/delete.rb:32) makes the guard explicit and side-effect-free.

Reproduction

The new test test_draw_block_returning_non_string_does_not_crash mirrors the minimal repro from the issue — a draw block that only assigns n.score = 42. Without the fix this raises RuntimeError: Can't find 'what' attribute out of [_id, _time, _version, score]. With the fix the new fact is inserted cleanly.

Verification

  • bundle exec rake test — 255 tests, 0 failures.
  • bundle exec rake rubocop — clean.
  • bundle exec rake yard — clean.
  • All 10 CI checks green on this PR.

Closes #445.

@0crat

0crat commented May 6, 2026

Copy link
Copy Markdown

@bibonix Hi! Your branch name "fix-conclude-roll-unset-attrs" doesn't follow our naming guidelines - we'd love to see you use the ticket number instead, like "452" for better tracking! This adjustment costs -6 points, bringing your score to +212. Keep up the great work and don't forget to check your Zerocracy account for updates! 🚀

@yegor256 yegor256 merged commit 08e350e into zerocracy:master May 6, 2026
10 checks passed
@0crat

0crat commented May 6, 2026

Copy link
Copy Markdown

@bibonix Thanks for the contribution! You've earned +20 points for this: +24 as a basis; -4 for contributing fewer than 30 hits-of-code (25 total). Please keep them coming! Your running score is +232; don't forget to check your Zerocracy account too.

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.

Fbe::Conclude#roll crashes with RuntimeError when draw block does not return a String

3 participants