Skip to content

fix: raise on offline tracker configuration errors - #1334

Open
davidberenstein1957 wants to merge 2 commits into
masterfrom
fix/suppress-constructor-failures
Open

fix: raise on offline tracker configuration errors#1334
davidberenstein1957 wants to merge 2 commits into
masterfrom
fix/suppress-constructor-failures

Conversation

@davidberenstein1957

@davidberenstein1957 davidberenstein1957 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Closes #1311

What changed

Removed @suppress(Exception) from OfflineEmissionsTracker.__init__ (codecarbon/emissions_tracker.py).

Why

The decorator swallowed configuration errors raised during construction, most visibly the OSError from _set_from_conf when output_dir does not exist. The constructor then returned an object that had never reached _initialize_runtime_state() / _initialize_scheduler_state(), so it had no _start_time, _hardware, _scheduler or _output_handlers. The subsequent start() and stop() calls are themselves suppressed, so they degraded into AttributeError warnings and the run finished with no emissions file and no exception. Under the CLI default log_level="error" nothing was printed at all.

The online EmissionsTracker already raises for the same input, so this also removes an asymmetry between the two constructors.

Suppression on start, flush and stop is deliberately left in place: runtime measurement errors must never crash a user job. Only construction, where the object invariants were never established, now fails loudly.

Verification

Added test_offline_tracker_raises_on_invalid_output_dir in tests/test_offline_emissions_tracker.py, asserting both the offline and the online tracker raise OSError for a non-existent output_dir. It fails on master and passes with this change.

uv run pytest tests/test_offline_emissions_tracker.py tests/test_emissions_tracker.py -q
35 passed

Note for reviewers

This is a behaviour change at a public boundary: code that today constructs a misconfigured offline tracker and carries on will now raise. That is the intended correction but it deserves a changelog entry, and ideally a minor release.

🤖 Generated with Claude Code

Changelog / release note (added)

CHANGELOG.md is not on master yet (it lands with docs/traction-batch), so the entry lives in the docs: docs/reference/api.md now carries a "Constructor errors (changed in v3.4.0)" warning. When the changelog lands, copy:

ChangedOfflineEmissionsTracker no longer swallows exceptions raised while constructing the tracker. Configuration errors (for example a non-existent output_dir) now propagate to the caller, matching EmissionsTracker. Suppression on start/flush/stop is unchanged.

This must ride a minor release (3.4.0), not a patch.

A second constructor-raise case is now covered: test_offline_tracker_raises_on_invalid_region exercises the offline-only region check, which runs before super().__init__ and so was suppressed by a different code path than the output_dir OSError. Both new tests fail when @suppress(Exception) is put back on the constructor.

`@suppress(Exception)` on `OfflineEmissionsTracker.__init__` swallowed
configuration errors such as a missing `output_dir`, returning a
half-built object with no `_start_time`, `_hardware` or `_scheduler`.
`start()`/`stop()` then failed silently and no emissions were recorded.

Construction now raises, matching `EmissionsTracker`. The suppression on
`start`/`flush`/`stop` is kept, so runtime measurement errors still
cannot crash a user's job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.47%. Comparing base (065d0e6) to head (85eedc8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1334      +/-   ##
==========================================
+ Coverage   91.39%   91.47%   +0.07%     
==========================================
  Files          49       49              
  Lines        5056     5055       -1     
==========================================
+ Hits         4621     4624       +3     
+ Misses        435      431       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davidberenstein1957
davidberenstein1957 marked this pull request as ready for review August 12, 2026 19:14
@davidberenstein1957
davidberenstein1957 requested a review from a team as a code owner August 12, 2026 19:14
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.

OfflineEmissionsTracker silently returns a half-built object when construction fails

1 participant