Skip to content

fix(api): raise on unsuccessful API responses - #1288

Closed
PSR94 wants to merge 1 commit into
mlco2:masterfrom
PSR94:contrib/fix-api-client-http-errors
Closed

fix(api): raise on unsuccessful API responses#1288
PSR94 wants to merge 1 commit into
mlco2:masterfrom
PSR94:contrib/fix-api-client-http-errors

Conversation

@PSR94

@PSR94 PSR94 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Make the CodeCarbon API client raise requests.exceptions.HTTPError when the API returns an unexpected HTTP status, while preserving the existing tracker-facing output handling that catches upload failures.

Why this helps

Callers can now distinguish API failures through standard requests exceptions instead of receiving None or False from failed HTTP responses. This supports clearer error handling while keeping emissions uploads from breaking tracker execution.

Changes made

  • Added a shared checked request helper in ApiClient that logs API response details and calls raise_for_status().
  • Updated API client methods to use the shared helper for expected HTTP status validation.
  • Let unsuccessful emission uploads and run creation HTTP responses raise HTTPError.
  • Updated API client tests to assert the new error contract.

Testing

  • uv run pytest tests/test_api_call.py tests/output_methods/test_http.py -q
  • uv run ruff check codecarbon/core/api_client.py tests/test_api_call.py tests/output_methods/test_http.py
  • uv run black --check --diff codecarbon/core/api_client.py tests/test_api_call.py tests/output_methods/test_http.py
  • CODECARBON_ALLOW_MULTIPLE_RUNS=True uv run pytest --ignore=tests/test_viz_data.py -q -m 'not integ_test' tests/

Closes #820

@PSR94
PSR94 requested a review from a team as a code owner August 2, 2026 20:29
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.37%. Comparing base (4afa249) to head (3356785).

Files with missing lines Patch % Lines
codecarbon/core/api_client.py 76.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1288      +/-   ##
==========================================
+ Coverage   89.70%   90.37%   +0.67%     
==========================================
  Files          48       48              
  Lines        4778     4718      -60     
==========================================
- Hits         4286     4264      -22     
+ Misses        492      454      -38     

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

@inimaz inimaz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR @PSR94 . Unfortunately as you might have been in the issue, it is not enough with throwing the error, one has to go into the places where this client has been called (cli and emissionsTracker) and do a try-catch around them.

There is another PR related to this here --> #1277.
I see 2 options either you go and try-catch them or you wait until that PR does it and once merged resolve the conflicts and use your _request function. What do you think?

headers["Authorization"] = f"Bearer {self.access_token}"
return headers

def _request(self, method, url, payload=None, expected_status=200):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like this helper!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indeed it elegant. I will do this in #1277 and will credit you @PSR94 in the release note.

I prefer to merge #1277 as it better address the problem as a whole.

@inimaz inimaz added enhancement New feature or request question Further information is requested labels Aug 3, 2026
benoit-cty pushed a commit to sankalpsthakur/codecarbon that referenced this pull request Aug 3, 2026
Every method repeated the same four lines: build the headers, call
requests, compare the status code, log and raise. They now all delegate
to a single `_request(method, url, payload, expected_status)`, which
removes 44 lines from the module.

Behaviour is unchanged: `_request` calls `_raise_api_error` on an
unexpected status, so the errors raised, the logs and the re-raise
clauses of `add_emission` and `_create_run` all stay as they were.

The helper comes from the alternative fix proposed in mlco2#1288, which
targets the same issue.

Co-authored-by: PSR94 <88868390+PSR94@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@benoit-cty benoit-cty closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API client to throw error instead of None

3 participants