fix: raise HTTP errors from ApiClient instead of returning None

[sankalpsthakur]

Description

ApiClient now calls response.raise_for_status() after logging HTTP failures instead of returning None / False / []. Callers get real requests.exceptions.HTTPErrors (and connection errors are re-raised from _create_run / add_emission).

EmissionsTracker wraps Code Carbon API output initialization in try/except so tracking continues without API reporting if setup fails. Removed the CLI organization is None guard that assumed silent failure, and dropped the organizations is None check in check_organization_exists.

Related Issue

Fixes #820

Motivation and Context

Silent None returns made API failures hard to debug and could confuse callers (e.g. iterating over None). The issue asks for raise_for_status in the client while keeping the tracker resilient.

Note: #1089 is an earlier approved attempt that is stale with failing CI and does not wrap tracker API init. Happy to close this in favor of a refreshed #1089 if maintainers prefer.

How Has This Been Tested?

CODECARBON_ALLOW_MULTIPLE_RUNS=True .venv/bin/python -m pytest \
  tests/test_api_call.py \
  tests/output_methods/test_http.py \
  tests/test_emissions_tracker.py::TestCarbonTracker::test_api_output_init_failure_does_not_break_tracker \
  tests/cli/test_cli_main.py \
  tests/cli/test_cli.py -q

Results: all targeted tests passed (26 for api/http/tracker slice; 24 CLI).

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Direct ApiClient callers that previously checked for None on HTTP errors will now see raised exceptions (intended). Tracker/live emission paths catch and log.

AI Usage Disclosure

Please refer to docs/how-to/ai-policy.md for detailed guidelines on how to disclose AI usage in your PR. Accurately completing this section is mandatory.

• 🟥 AI-vibecoded: You cannot explain the logic. Car analogy : the car drive by itself, you are outside it and just tell it where to go.
• 🟠 AI-generated: Car analogy : the car drive by itself, you are inside and give instructions.
• ⭐ AI-assisted. Car analogy : you drive the car, AI help you find your way.
• ♻️ No AI used. Car analogy : you drive the car.

AI coding tools were used to help draft the change and this PR description. I reviewed the complete change, understand the reasoning, and ran the reported local tests before submitting.

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the docs/how-to/contributing.md document.
• I have added tests to cover my changes.
• All new and existing tests passed.

Made with Cursor

[sankalpsthakur]

Cross-linked on #1089. Prefer maintainers pick one of the two rather than both landing.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.43%. Comparing base (2cf5881) to head (93e72ad).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1277      +/-   ##
==========================================
+ Coverage   90.39%   91.43%   +1.03%     
==========================================
  Files          49       49              
  Lines        5103     5056      -47     
==========================================
+ Hits         4613     4623      +10     
+ Misses        490      433      -57     

☔ 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.

[benoit-cty]

Thanks for you contribution. It seems it need some improvements.

Here is the review of Opus 5:

Overview

Turns ApiClient's silent None/False/[] returns on HTTP failure into real exceptions by adding response.raise_for_status() at the end of _log_error, re-raising connection/generic errors from _create_run and add_emission, and dropping the now-dead None guards in check_organization_exists and the CLI. EmissionsTracker._init_output_methods wraps API-output construction in try/except so tracking survives an unreachable API. Direction is right and matches what #820 asked for.

I verified the propagation paths: http.py:51-57 uses create_run_automatically=False and _emit already catches everything, and all CLI ApiClient(...) constructions pass no experiment_id — so no constructor now raises except through the tracker path the PR explicitly guards. The new tracker test's patch target is also correct, because emissions_tracker.py:618 imports CodeCarbonAPIOutput inside the function, so mock.patch("codecarbon.output_methods.http.CodeCarbonAPIOutput") resolves at call time.

---

Issues

1. raise_for_status() doesn't cover the condition the guard actually tests (correctness — should fix before merge)

Every call site guards on if r.status_code != 200 / != 201, but raise_for_status() only raises for >= 400. For any unexpected 2xx/3xx, _log_error now logs and returns normally, and execution falls through into code that previously was unreachable:

• api_client.py:238-241 — a 200/202 on POST /emissions now logs an error and then returns True (success!) instead of False.
• api_client.py:280-283 — a 200 on POST /runs falls into r.json()["id"] → KeyError, or JSONDecodeError on a 204.
• The plain getters (get_experiment, get_project, get_list_organizations, …) return r.json() on a 204/non-JSON body → JSONDecodeError leaking to the CLI.

Fix by raising unconditionally for any status the caller didn't expect:

def _log_error(self, url, payload, response):
    ...
    logger.error(f"ApiClient API return http code {response.status_code} ...")
    response.raise_for_status()
    # 2xx/3xx that still isn't what the caller expected
    raise requests.exceptions.HTTPError(
        f"Unexpected status {response.status_code} from {url}", response=response
    )

2. _log_error raising is hidden control flow (design)

A method named _log_error that terminates the caller makes every call site misleading — self._log_error(...) followed by return r.json() reads as fall-through but is dead code, which is precisely how issue 1 slipped in. Either rename it (_raise_api_error / _handle_error) or keep it pure and put an explicit r.raise_for_status() at each site. The rename is cheap and makes the diff self-documenting.

3. CLI commands now emit raw tracebacks (UX regression)

• main.py:133 — codecarbon login against a bad token: check_auth() raised HTTPError, unhandled.
• main.py:119 — codecarbon test-api on any API error: unhandled.
• main.py:215 — codecarbon config wizard: get_list_organizations() now raises instead of yielding the old TypeError: 'NoneType' is not iterable. Better than before, but still a stack trace in an interactive wizard.

Only show_config has a try/except (main.py:100-103). Since the PR is already touching this file, wrapping these three in the same friendly-message pattern would keep the CLI usable.

4. Removed one dead guard but left the sibling ones

main.py:233-235 was removed, yet the equivalent defensive guards remain at:

• main.py:243 (if projects else [])
• main.py:268 (if experiments else [])

Harmless, but now inconsistent — clean both for the same reason.

5. Double logging on every failed emission (minor)

In add_emission/_create_run, _log_error logs the URL + response body, then raises, then the enclosing except Exception as e: logger.error(e, exc_info=True); raise logs it again with a traceback. add_emission fires once per measurement interval, so a persistently failing API doubles the log volume. Consider narrowing to except requests.exceptions.HTTPError: raise before the generic handler.

6. Redundant tracker branch (nit)

self.run_id = uuid.uuid4() in the new except duplicates the else at emissions_tracker.py:649. Fine as-is; a finally-style fallback (if self.run_id is None: self.run_id = uuid.uuid4()) would be tighter.

---

Tests

Test updates are faithful and the imports check out (requests added to test_api_call.py; requests and CodeCarbonAPIOutput already present in test_emissions_tracker.py). Gaps:

• No test for the 2xx-but-unexpected case — the exact hole in issue 1. Add e.g. m.post("http://test.com/runs", json={}, status_code=200) and assert it raises rather than KeyError-ing.
• test_api_output_init_failure_does_not_break_tracker would also pass if the patch silently didn't apply (real construction → connection error → same except). Asserting the mock was called would make it prove the intended path.
• No test for the new CLI behaviour on API failure.
• codecov/patch is red — worth checking which new lines are uncovered.

---

Other

• Correctly flagged as a breaking change, but the checklist marks no docs update. ApiClient is public API; a note in the docs/release notes for anyone catching None today would help.
• On the PR's own question about fix(api_client): raise HTTPError instead of returning None on API errors #1089: this one is more complete (it wraps tracker init, fix(api_client): raise HTTPError instead of returning None on API errors #1089 doesn't). Preferring this and closing fix(api_client): raise HTTPError instead of returning None on API errors #1089 seems right.

---

[sankalpsthakur]

Rebased onto master. The 2 codecov/patch misses are the defensive except branch in emissions_tracker.py — exercised locally by test_api_output_init_failure_does_not_break_tracker (run_id still set, no handler appended). 48 api/tracker tests pass.

[sankalpsthakur]

Thanks @benoit-cty — addressed the Opus review:

1. _raise_api_error (renamed from _log_error) now always raises after logging, including unexpected 2xx/3xx.
2. CLI login / test-api / config catch API failures and print a friendly message instead of a raw traceback.
3. Dropped the leftover if projects/experiments else [] guards.
4. HTTPError from _raise_api_error is re-raised without a second log in add_emission / _create_run.
5. Added tests for unexpected 2xx on create-run/add-emission, CLI failure messages, and asserting the tracker API-output mock was called.

Skipped the tracker run_id nit — run_id is not always an attribute there, so the is None fallback AttributeErrors.

[benoit-cty]

Thanks for fixing all this.

[benoit-cty]

I've took a good idea from #1288 to reduce the line of code with _request. Thanks to @PSR94 .

[PSR94]

Thanks for the review and for the credit. Glad the helper approach was useful.