Skip to content

fix(logger): flush rolling buffer when executor errors#352

Merged
not-matthias merged 1 commit into
mainfrom
cod-2665-showing-output-in-box-hides-error-messages
May 14, 2026
Merged

fix(logger): flush rolling buffer when executor errors#352
not-matthias merged 1 commit into
mainfrom
cod-2665-showing-output-in-box-hides-error-messages

Conversation

@not-matthias
Copy link
Copy Markdown
Member

Summary

  • When run_executor returns Err, the ? short-circuit was skipping deactivate_rolling_buffer(). The rolling buffer stayed active in its global static, so both the deferred profiler warning (e.g. "Walltime profiling is enabled, but failed to detect benchmarks") and the final error printed by main via log::error! ended up queued in DEFERRED_LOGS and never flushed — the user saw the box close and the prompt return with no indication of what went wrong.
  • Capture the result, run deactivate_rolling_buffer() unconditionally, then propagate. finish() triggers flush_deferred_logs(), so queued warnings render above the final box, and by the time main calls log::error! the rolling buffer is None and the error prints normally.

Closes COD-2665

Test plan

  • cargo r -- run --mode walltime -- ls shows the "Walltime profiling is enabled, but failed to detect benchmarks" warning and the "did not produce any CodSpeed result" error after the box closes
  • cargo r -- run --show-full-output --mode walltime -- ls behavior unchanged
  • Successful runs still tear the box down cleanly with a checkmark

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 14, 2026

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks


Comparing cod-2665-showing-output-in-box-hides-error-messages (149a618) with main (9afaef7)

Open in CodSpeed

Copy link
Copy Markdown
Contributor

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

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

LGTM to me

When run_executor returned Err, the `?` short-circuit skipped
deactivate_rolling_buffer(), leaving the rolling buffer active in its
global static. Both the deferred profiler warning and the final error
printed by main via log::error! got swallowed by the deferred-logs
queue, hiding the actual failure cause from the user.

Capture the result, always deactivate, then propagate.

Closes COD-2665
@not-matthias not-matthias force-pushed the cod-2665-showing-output-in-box-hides-error-messages branch from 1649fe4 to 149a618 Compare May 14, 2026 22:02
@not-matthias not-matthias merged commit 149a618 into main May 14, 2026
21 checks passed
@not-matthias not-matthias deleted the cod-2665-showing-output-in-box-hides-error-messages branch May 14, 2026 22:42
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.

2 participants