fix: raise HTTP errors from ApiClient instead of returning None

codecarbon/cli/main.py

@@ -40,6 +40,20 @@ def main():
         raise sys.exit(1)
 
 
+def _api_call(action: str, func, *args, **kwargs):
+    """
+    Run a call to the Code Carbon API, turning the errors it now raises into a
+    readable message and a clean exit instead of a traceback.
+
+    :action: what was being attempted, used as the first part of the message.
+    """
+    try:
+        return func(*args, **kwargs)
+    except Exception as e:
+        print(f"[yellow]{action}[/yellow]. (error: {e})")
+        raise typer.Exit(1)
+
+
 def _version_callback(value: bool) -> None:
     if value:
         print(f"{__app_name__} v{__version__}")
@@ -116,7 +130,7 @@ def api_get():
     api_endpoint = get_api_endpoint()
     api = ApiClient(endpoint_url=api_endpoint)
     api.set_access_token(get_access_token())
-    organizations = api.get_list_organizations()
+    organizations = _api_call("API request failed", api.get_list_organizations)
     print(organizations)
 
 
@@ -130,7 +144,7 @@ def login():
     api = ApiClient(endpoint_url=api_endpoint)
     access_token = get_access_token()
     api.set_access_token(access_token)
-    api.check_auth()
+    _api_call("Authentication check failed", api.check_auth)
 
 
 def get_api_key(project_id: str):
@@ -149,6 +163,7 @@ def get_api_key(project_id: str):
         },
         headers={"Authorization": f"Bearer {get_access_token()}"},
     )
+    req.raise_for_status()
     api_key = req.json()["token"]
     return api_key
 
@@ -212,7 +227,10 @@ def config():
     overwrite_local_config("api_endpoint", api_endpoint, path=file_path)
     api = ApiClient(endpoint_url=api_endpoint)
     api.set_access_token(get_access_token())
-    organizations = api.get_list_organizations()
+    organizations = _api_call(
+        "Could not list organizations from API. Please check your login and API endpoint",
+        api.get_list_organizations,
+    )
     org = questionary_prompt(
         "Pick existing organization from list or Create new organization ?",
         [org["name"] for org in organizations] + ["Create New Organization"],
@@ -229,18 +247,23 @@ def config():
             name=org_name,
             description=org_description,
         )
-        organization = api.create_organization(organization=organization_create)
-        if organization is None:
-            print("Error creating organization")
-            return
+        organization = _api_call(
+            "Could not create the organization",
+            api.create_organization,
+            organization=organization_create,
+        )
         print(f"Created organization : {organization}")
     else:
         organization = [orga for orga in organizations if orga["name"] == org][0]
     org_id = organization["id"]
     overwrite_local_config("organization_id", org_id, path=file_path)
 
-    projects = api.list_projects_from_organization(org_id)
-    project_names = [project["name"] for project in projects] if projects else []
+    projects = _api_call(
+        "Could not list projects from API",
+        api.list_projects_from_organization,
+        org_id,
+    )
+    project_names = [project["name"] for project in projects]
     project = questionary_prompt(
         "Pick existing project from list or Create new project ?",
         project_names + ["Create New Project"],
@@ -256,17 +279,21 @@ def config():
             description=project_description,
             organization_id=org_id,
         )
-        project = api.create_project(project=project_create)
+        project = _api_call(
+            "Could not create the project", api.create_project, project=project_create
+        )
         print(f"Created project : {project}")
     else:
         project = [p for p in projects if p["name"] == project][0]
     project_id = project["id"]
     overwrite_local_config("project_id", project_id, path=file_path)
 
-    experiments = api.list_experiments_from_project(project_id)
-    experiments_names = (
-        [experiment["name"] for experiment in experiments] if experiments else []
+    experiments = _api_call(
+        "Could not list experiments from API",
+        api.list_experiments_from_project,
+        project_id,
     )
+    experiments_names = [experiment["name"] for experiment in experiments]
 
     experiment = questionary_prompt(
         "Pick existing experiment from list or Create new experiment ?",
@@ -313,13 +340,17 @@ def config():
             cloud_provider=cloud_provider,
             cloud_region=cloud_region,
         )
-        experiment = api.add_experiment(experiment=experiment_create)
+        experiment = _api_call(
+            "Could not create the experiment",
+            api.add_experiment,
+            experiment=experiment_create,
+        )
 
     else:
         experiment = [e for e in experiments if e["name"] == experiment][0]
 
     overwrite_local_config("experiment_id", experiment["id"], path=file_path)
-    api_key = get_api_key(project_id)
+    api_key = _api_call("Could not get the project API key", get_api_key, project_id)
     overwrite_local_config("api_key", api_key, path=file_path)
     show_config(file_path)
     print(

codecarbon/core/api_client.py

@@ -70,6 +70,21 @@ def _get_headers(self):
             headers["Authorization"] = f"Bearer {self.access_token}"
         return headers
 
+    def _request(self, method, url, payload=None, expected_status=200):
+        """
+        Call the API and return the response, raising on anything that is not
+        the status code the API answers on success.
+
+        :method: the requests function to call, for example requests.get
+        :payload: the JSON body to send, if any
+        :expected_status: the http code the API returns when the call succeeds
+        """
+        headers = self._get_headers()
+        response = method(url=url, json=payload, timeout=2, headers=headers)
+        if response.status_code != expected_status:
+            self._raise_api_error(url, payload or {}, response)
+        return response
+
     def set_access_token(self, token: str):
         """This method sets the access token to be used for the API.
         Args:
@@ -82,32 +97,20 @@ def check_auth(self):
         Check API access to user account
         """
         url = self.url + "/auth/check"
-        headers = self._get_headers()
-        r = requests.get(url=url, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, {}, r)
-            return None
-        return r.json()
+        return self._request(requests.get, url).json()
 
     def get_list_organizations(self):
         """
         List all organizations
         """
         url = self.url + "/organizations"
-        headers = self._get_headers()
-        r = requests.get(url=url, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, {}, r)
-            return None
-        return r.json()
+        return self._request(requests.get, url).json()
 
     def check_organization_exists(self, organization_name: str):
         """
         Check if an organization exists
         """
         organizations = self.get_list_organizations()
-        if organizations is None:
-            return False
         for organization in organizations:
             if organization["name"] == organization_name:
                 return organization
@@ -125,74 +128,48 @@ def create_organization(self, organization: OrganizationCreate):
             )
             return organization
         else:
-            headers = self._get_headers()
-            r = requests.post(url=url, json=payload, timeout=2, headers=headers)
-            if r.status_code != 201:
-                self._log_error(url, payload, r)
-                return None
-            return r.json()
+            return self._request(
+                requests.post, url, payload=payload, expected_status=201
+            ).json()
 
     def get_organization(self, organization_id):
         """
         Get an organization
         """
-        headers = self._get_headers()
         url = self.url + "/organizations/" + organization_id
-        r = requests.get(url=url, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, {}, r)
-            return None
-        return r.json()
+        return self._request(requests.get, url).json()
 
     def update_organization(self, organization: OrganizationCreate):
         """
         Update an organization
         """
         payload = dataclasses.asdict(organization)
-        headers = self._get_headers()
         url = self.url + "/organizations/" + organization.id
-        r = requests.patch(url=url, json=payload, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, payload, r)
-            return None
-        return r.json()
+        return self._request(requests.patch, url, payload=payload).json()
 
     def list_projects_from_organization(self, organization_id):
         """
         List all projects
         """
         url = self.url + "/organizations/" + organization_id + "/projects"
-        headers = self._get_headers()
-        r = requests.get(url=url, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, {}, r)
-            return None
-        return r.json()
+        return self._request(requests.get, url).json()
 
     def create_project(self, project: ProjectCreate):
         """
         Create a project
         """
         payload = dataclasses.asdict(project)
         url = self.url + "/projects"
-        headers = self._get_headers()
-        r = requests.post(url=url, json=payload, timeout=2, headers=headers)
-        if r.status_code != 201:
-            self._log_error(url, payload, r)
-            return None
-        return r.json()
+        return self._request(
+            requests.post, url, payload=payload, expected_status=201
+        ).json()
 
     def get_project(self, project_id):
         """
         Get a project
         """
         url = self.url + "/projects/" + project_id
-        headers = self._get_headers()
-        r = requests.get(url=url, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, {}, r)
-            return None
-        return r.json()
+        return self._request(requests.get, url).json()
 
     def add_emission(self, carbon_emission: dict):
         assert self.experiment_id is not None
@@ -233,15 +210,14 @@ def add_emission(self, carbon_emission: dict):
         try:
             payload = dataclasses.asdict(emission)
             url = self.url + "/emissions"
-            headers = self._get_headers()
-            r = requests.post(url=url, json=payload, timeout=2, headers=headers)
-            if r.status_code != 201:
-                self._log_error(url, payload, r)
-                return False
+            self._request(requests.post, url, payload=payload, expected_status=201)
             logger.debug(f"ApiClient - Successful upload emission {payload} to {url}")
+        except requests.exceptions.HTTPError:
+            # Already logged by _raise_api_error, do not log it twice.
+            raise
         except Exception as e:
             logger.error(e, exc_info=True)
-            return False
+            raise
         return True
 
     def _create_run(self, experiment_id: str):
@@ -275,11 +251,7 @@ def _create_run(self, experiment_id: str):
             )
             payload = dataclasses.asdict(run)
             url = self.url + "/runs"
-            headers = self._get_headers()
-            r = requests.post(url=url, json=payload, timeout=2, headers=headers)
-            if r.status_code != 201:
-                self._log_error(url, payload, r)
-                return None
+            r = self._request(requests.post, url, payload=payload, expected_status=201)
             self.run_id = r.json()["id"]
             logger.info(
                 "ApiClient Successfully registered your run on the API.\n\n"
@@ -292,20 +264,20 @@ def _create_run(self, experiment_id: str):
                 f"Failed to connect to API, please check the configuration. {e}",
                 exc_info=False,
             )
+            raise
+        except requests.exceptions.HTTPError:
+            # Already logged by _raise_api_error, do not log it twice.
+            raise
         except Exception as e:
             logger.error(e, exc_info=True)
+            raise
 
     def list_experiments_from_project(self, project_id: str):
         """
         List all experiments for a project
         """
         url = self.url + "/projects/" + project_id + "/experiments"
-        headers = self._get_headers()
-        r = requests.get(url=url, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, {}, r)
-            return []
-        return r.json()
+        return self._request(requests.get, url).json()
 
     def set_experiment(self, experiment_id: str):
         """
@@ -320,26 +292,21 @@ def add_experiment(self, experiment: ExperimentCreate):
         """
         payload = dataclasses.asdict(experiment)
         url = self.url + "/experiments"
-        headers = self._get_headers()
-        r = requests.post(url=url, json=payload, timeout=2, headers=headers)
-        if r.status_code != 201:
-            self._log_error(url, payload, r)
-            return None
-        return r.json()
+        return self._request(
+            requests.post, url, payload=payload, expected_status=201
+        ).json()
 
     def get_experiment(self, experiment_id):
         """
         Get an experiment by id
         """
         url = self.url + "/experiments/" + experiment_id
-        headers = self._get_headers()
-        r = requests.get(url=url, timeout=2, headers=headers)
-        if r.status_code != 200:
-            self._log_error(url, {}, r)
-            return None
-        return r.json()
+        return self._request(requests.get, url).json()
 
-    def _log_error(self, url, payload, response):
+    def _raise_api_error(self, url, payload, response):
+        """
+        Log the failed call then always raise a requests.exceptions.HTTPError.
+        """
         if len(payload) > 0:
             logger.error(
                 f"ApiClient Error when calling the API on {url} with : {json.dumps(payload)}"
@@ -349,6 +316,11 @@ def _log_error(self, url, payload, response):
         logger.error(
             f"ApiClient API return http code {response.status_code} and answer : {response.text}"
         )
+        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
+        )
 
     def close_experiment(self):
         """