Skip to content

fix: raise HTTP errors in ApiClient instead of returning None (#820)#1128

Closed
agodianel wants to merge 9 commits intomlco2:masterfrom
agodianel:fix/820-api-client-throw-errors
Closed

fix: raise HTTP errors in ApiClient instead of returning None (#820)#1128
agodianel wants to merge 9 commits intomlco2:masterfrom
agodianel:fix/820-api-client-throw-errors

Conversation

@agodianel
Copy link
Copy Markdown
Contributor

Resolves #820 by raising requests.exceptions.HTTPError when the API returns a non-success status code, rather than failing silently and returning None. This allows the EmissionsTracker to catch the exception and fall back to offline mode without crashing the user's execution run.

Description

Fixes #820

The ApiClient previously returned None or False on HTTP errors, which failed silently for the user but occasionally crashed upstream logic (e.g. attempting to read api.run_id when it was None). As requested in the issue, this PR updates the client to throw requests.exceptions.HTTPError on bad status codes and handles the exception upstream to fall back gracefully.

Changes:

  • Replaced return None/False/[] with r.raise_for_status() in all ApiClient data methods.
  • Re-raised exceptions in generic try...except blocks within ApiClient (e.g., inside _create_run).
  • Wrapped CodeCarbonAPIOutput initialization in emissions_tracker.py with a try...except block so that if the API is unreachable, the execution falls back to local tracking instead of tearing down the user's workload.
  • Added test_call_api_error_raises to test_api_call.py to ensure that 500 error responses raise the correct HTTP error natively.

All local CodeCarbon unit tests (including the new ones) execute and pass successfully.

)

Resolves mlco2#820 by raising requests.exceptions.HTTPError when the API returns
a non-success status code, rather than failing silently and returning None.
This allows the EmissionsTracker to catch the exception and fall back to
offline mode without crashing the user's execution run.
@agodianel agodianel requested a review from a team as a code owner March 23, 2026 00:17
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.71%. Comparing base (6356227) to head (cde5d87).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1128      +/-   ##
==========================================
+ Coverage   83.10%   85.71%   +2.61%     
==========================================
  Files          45       45              
  Lines        4279     4285       +6     
==========================================
+ Hits         3556     3673     +117     
+ Misses        723      612     -111     

☔ View full report in Codecov by Sentry.
📢 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.

@SaboniAmine
Copy link
Copy Markdown
Member

Hello, sorry to warn you only now, but #1089 already solves this issue, and we will likely merge it soon.

@agodianel
Copy link
Copy Markdown
Contributor Author

@SaboniAmine that's great!

@agodianel agodianel closed this Mar 30, 2026
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.

API client to throw error instead of None

2 participants