From ec0f5c9047010e63e87f94ca02be51e281fa942d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rnar=20Snoksrud?= Date: Thu, 11 Jun 2026 13:41:46 +0200 Subject: [PATCH 1/2] sdk: raise NovemException on API errors instead of a print placeholder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every resource write path (vis/group/job/repo `create_*` PUT and `api_write` POST) handled 404/403/409 but, on any other non-ok response, printed the body + headers + the literal "should raise a general error" and returned — so the caller got no exception and silently no-op'd. Terje hit exactly this (novem-code/gaia#2656): `mail.bcc = [over-cap list]` returned without error while nothing was stored. The server now rejects over-cap / unresolvable writes with a 400 + `{message, rejected:[…]}`, but the SDK swallowed it. Add `raise_on_response(r)` (api_ref) — parses the server `message` and any `rejected` lines and raises the matching NovemException subclass (401/403/ 404/409 → generic NovemException otherwise) — and call it from all eight write sites. `mail.bcc = …` over the cap now raises NovemException with the cap reason and the rejected addresses. --- novem/api_ref.py | 33 +++++++++++++++++++++++++++++++++ novem/exceptions/__init__.py | 20 ++++++++++++++++++-- novem/group/__init__.py | 29 +++-------------------------- novem/job/__init__.py | 29 +++-------------------------- novem/repo/__init__.py | 29 +++-------------------------- novem/vis/__init__.py | 29 +++-------------------------- 6 files changed, 63 insertions(+), 106 deletions(-) diff --git a/novem/api_ref.py b/novem/api_ref.py index 66998a2..3af1381 100644 --- a/novem/api_ref.py +++ b/novem/api_ref.py @@ -67,6 +67,39 @@ class NovemAuthError(NovemException): pass +def raise_on_response(r: requests.Response) -> None: + """Raise the appropriate ``NovemException`` for a non-ok API response. + + Surfaces the server's ``message`` and any ``rejected`` lines (e.g. the + recipient-write reject contract) so callers get a real exception to catch + instead of a silent no-op. No-op on a successful response. + """ + if r.ok or r.status_code == 409: + return + + try: + resp = r.json() + except ValueError: + resp = {} + + message = resp.get("message") or r.text or f"HTTP {r.status_code}" + + rejected = resp.get("rejected") + if rejected: + lines = "; ".join(f'{item.get("line")}: {item.get("reason")}' for item in rejected if isinstance(item, dict)) + if lines: + message = f"{message} [{lines}]" + + code = r.status_code + if code == 401: + raise Novem401(message) + if code == 403: + raise Novem403(message) + if code == 404: + raise Novem404(message) + raise NovemException(message) + + class NovemAPI(object): """ Novem API class diff --git a/novem/exceptions/__init__.py b/novem/exceptions/__init__.py index a79a7c1..06fd3bd 100644 --- a/novem/exceptions/__init__.py +++ b/novem/exceptions/__init__.py @@ -1,3 +1,19 @@ -from ..api_ref import Novem401, Novem403, Novem404, NovemAuthError, NovemException +from ..api_ref import ( + Novem401, + Novem403, + Novem404, + Novem409, + NovemAuthError, + NovemException, + raise_on_response, +) -__all__ = ["NovemException", "Novem404", "Novem403", "Novem401", "NovemAuthError"] +__all__ = [ + "NovemException", + "Novem404", + "Novem403", + "Novem401", + "Novem409", + "NovemAuthError", + "raise_on_response", +] diff --git a/novem/group/__init__.py b/novem/group/__init__.py index 0bbb712..3492ac1 100644 --- a/novem/group/__init__.py +++ b/novem/group/__init__.py @@ -1,6 +1,6 @@ from typing import Any, Optional -from novem.exceptions import Novem403, Novem404 +from novem.exceptions import Novem403, Novem404, raise_on_response from ..api_ref import NovemAPI from .profile import NovemGroupProfile @@ -147,17 +147,8 @@ def api_create(self, relpath: str) -> None: # as creating objects that already exist is not a problem return - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"PUT: {path}") - print("body") - print("---") - print(r.text) - print("headers") - print("---") - print(r.headers) - print("should raise a general error") + raise_on_response(r) def api_write(self, relpath: str, value: str) -> None: """ @@ -180,22 +171,8 @@ def api_write(self, relpath: str, value: str) -> None: if r.status_code == 404: raise Novem404(path) - if r.status_code == 403: - raise Novem403 - - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"POST: {path} {value}") - print("body") - print("---") - print(r.text) - print(r.status_code) - print("headers") - print("---") - for k, v in r.headers.items(): - print(f" {k}: {v}") - print("should raise a general error") + raise_on_response(r) @property def permissions(self) -> str: diff --git a/novem/job/__init__.py b/novem/job/__init__.py index ad94e50..533372f 100644 --- a/novem/job/__init__.py +++ b/novem/job/__init__.py @@ -3,7 +3,7 @@ import sys from typing import Any, Dict, List, Optional, Tuple -from novem.exceptions import Novem403, Novem404 +from novem.exceptions import Novem403, Novem404, raise_on_response from ..api_ref import NovemAPI from ..shared import NovemShare @@ -182,17 +182,8 @@ def api_create(self, relpath: str) -> None: # as creating objects that already exist is not a problem return - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"PUT: {path}") - print("body") - print("---") - print(r.text) - print("headers") - print("---") - print(r.headers) - print("should raise a general error") + raise_on_response(r) def api_write(self, relpath: str, value: str) -> None: """ @@ -218,22 +209,8 @@ def api_write(self, relpath: str, value: str) -> None: if r.status_code == 404: raise Novem404(path) - if r.status_code == 403: - raise Novem403 - - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"POST: {path} {value}") - print("body") - print("---") - print(r.text) - print(r.status_code) - print("headers") - print("---") - for k, v in r.headers.items(): - print(f" {k}: {v}") - print("should raise a general error") + raise_on_response(r) # chainable utility function for setting values def w(self, key: str, value: str) -> Any: diff --git a/novem/repo/__init__.py b/novem/repo/__init__.py index 52254d6..d0cc2c5 100644 --- a/novem/repo/__init__.py +++ b/novem/repo/__init__.py @@ -1,6 +1,6 @@ from typing import Any, Optional -from novem.exceptions import Novem403, Novem404 +from novem.exceptions import Novem403, Novem404, raise_on_response from ..api_ref import NovemAPI from ..shared import NovemShare @@ -144,17 +144,8 @@ def api_create(self, relpath: str) -> None: # as creating objects that already exist is not a problem return - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"PUT: {path}") - print("body") - print("---") - print(r.text) - print("headers") - print("---") - print(r.headers) - print("should raise a general error") + raise_on_response(r) def api_write(self, relpath: str, value: str) -> None: """ @@ -177,22 +168,8 @@ def api_write(self, relpath: str, value: str) -> None: if r.status_code == 404: raise Novem404(path) - if r.status_code == 403: - raise Novem403 - - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"POST: {path} {value}") - print("body") - print("---") - print(r.text) - print(r.status_code) - print("headers") - print("---") - for k, v in r.headers.items(): - print(f" {k}: {v}") - print("should raise a general error") + raise_on_response(r) # chainable utility function for setting values def w(self, key: str, value: str) -> Any: diff --git a/novem/vis/__init__.py b/novem/vis/__init__.py index 7a28121..f49bab4 100644 --- a/novem/vis/__init__.py +++ b/novem/vis/__init__.py @@ -2,7 +2,7 @@ import warnings from typing import Any, Dict, List, Optional, Tuple -from novem.exceptions import Novem403, Novem404 +from novem.exceptions import Novem403, Novem404, raise_on_response from ..api_ref import NovemAPI from ..shared import NovemShare @@ -383,17 +383,8 @@ def api_create(self, relpath: str) -> None: # as creating objects that already exist is not a problem return - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"PUT: {path}") - print("body") - print("---") - print(r.text) - print("headers") - print("---") - print(r.headers) - print("should raise a general error") + raise_on_response(r) def api_write(self, relpath: str, value: str) -> None: """ @@ -419,22 +410,8 @@ def api_write(self, relpath: str, value: str) -> None: if r.status_code == 404: raise Novem404(path) - if r.status_code == 403: - raise Novem403 - - # TODO: verify result and raise exception if not ok if not r.ok: - print(r) - print(f"POST: {path} {value}") - print("body") - print("---") - print(r.text) - print(r.status_code) - print("headers") - print("---") - for k, v in r.headers.items(): - print(f" {k}: {v}") - print("should raise a general error") + raise_on_response(r) @property def log(self) -> None: From 347a12dc43581fa43358237413732cefbb8aed4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rnar=20Snoksrud?= Date: Wed, 17 Jun 2026 12:00:59 +0200 Subject: [PATCH 2/2] style: apply isort to exceptions __init__ --- novem/exceptions/__init__.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/novem/exceptions/__init__.py b/novem/exceptions/__init__.py index 06fd3bd..37f8eb2 100644 --- a/novem/exceptions/__init__.py +++ b/novem/exceptions/__init__.py @@ -1,12 +1,4 @@ -from ..api_ref import ( - Novem401, - Novem403, - Novem404, - Novem409, - NovemAuthError, - NovemException, - raise_on_response, -) +from ..api_ref import Novem401, Novem403, Novem404, Novem409, NovemAuthError, NovemException, raise_on_response __all__ = [ "NovemException",