refactor: extract API error handling into decorator#352
Closed
sorlen008 wants to merge 2 commits intocyberjunky:masterfrom
Closed
refactor: extract API error handling into decorator#352sorlen008 wants to merge 2 commits intocyberjunky:masterfrom
sorlen008 wants to merge 2 commits intocyberjunky:masterfrom
Conversation
connectapi(), connectwebproxy(), and download() each contained nearly
identical error-handling blocks — roughly 50 lines of duplicated logic
mapping HTTPError / GarminConnectConnectionError to domain exceptions
based on HTTP status.
Extracted that into a single _handle_api_errors(label) decorator factory.
Each wrapped method now holds just the one-line call to self.client.
Exception types and status mapping are unchanged; log messages keep the
per-method label ('API call', 'Web proxy call', 'Download').
Net change: -78 lines, +54 lines. Makes it easier to add retry or other
cross-cutting concerns later without touching each method.
Contributor
WalkthroughIntroduces a reusable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@garminconnect/__init__.py`:
- Around line 102-118: The decorator _handle_api_errors currently types
everything as Callable which loses the wrapped function signature; update its
typing to use typing.ParamSpec and a TypeVar for the return type so the
decorator preserves parameter and return info: add "P = ParamSpec('P')" and "R =
TypeVar('R')" to imports, change the decorator signature to "def
_handle_api_errors(label: str) -> Callable[[Callable[P, R]], Callable[P, R]]"
(or use def decorator(func: Callable[P, R]) -> Callable[P, R]) and type the
inner functions accordingly (reference symbols: _handle_api_errors, decorator,
func) so IDEs and static checkers retain exact call signatures.
- Around line 140-142: The generic except Exception in the error handling around
the request (in the block that logs "Connection error during %s path=%s" and
raises GarminConnectConnectionError) is catching and wrapping domain-specific
exceptions from _run_request (notably GarminConnectTooManyRequestsError and
GarminConnectAuthenticationError), breaking callers' ability to handle them.
Change the handler to only catch actual connection/HTTP-related exceptions
(e.g., requests.exceptions.RequestException or HTTPError) or explicitly re-raise
domain exceptions from GarminConnectTooManyRequestsError and
GarminConnectAuthenticationError so they propagate unchanged; update the except
clause around logger.exception/raise GarminConnectConnectionError in __init__.py
accordingly and reference _run_request in client.py to ensure domain errors are
not wrapped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d9223405-0b22-4c08-b1af-ed29d682b757
📒 Files selected for processing (1)
garminconnect/__init__.py
The previous `_handle_api_errors` decorator had a trailing `except Exception` clause that swallowed the domain-specific `GarminConnectAuthenticationError` and `GarminConnectTooManyRequestsError` exceptions raised directly by `client._run_request` and the login flow, re-wrapping them as generic `GarminConnectConnectionError` and losing the specific error type. Catch these two Garmin-specific exceptions first and re-raise them unchanged so callers can still distinguish 401/429 from generic connection errors. HTTPError / GarminConnectConnectionError handling (including status-code mapping) is unchanged. Also type the decorator with `ParamSpec` + `TypeVar` so wrapped methods keep their exact signature for type-checkers, and add `tests/test_error_handler_decorator.py` with 9 regression tests covering the passthrough case, status-code mapping, and generic exception wrapping. Addresses CodeRabbit review on PR cyberjunky#352.
Contributor
Author
|
Thanks @coderabbitai for catching the critical issue with exception handling — you were absolutely right. The bugThe decorator's trailing The fix (commit e32fb1c)Catch the two Garmin-specific exceptions first and re-raise them unchanged: def decorator(func: Callable[P, R]) -> Callable[P, R]:
@functools.wraps(func)
def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
path = args[1] if len(args) > 1 else kwargs.get("path", "<unknown>")
try:
return func(*args, **kwargs)
except (
GarminConnectAuthenticationError,
GarminConnectTooManyRequestsError,
):
# Already domain-specific — let them propagate unchanged.
raise
except (HTTPError, GarminConnectConnectionError) as e:
status = getattr(getattr(e, "response", None), "status_code", None)
... # unchanged 401/429/4xx/other mapping
except Exception as e:
... # unchanged generic wrapI also applied the VerificationAdded
Full suite: 28 passed, 2 skipped (the two skips are the pre-existing live Garmin VCR tests). |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
The test suite additions are particularly thorough — The If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_error_handler_decorator.py`:
- Around line 36-133: Remove the return type hints ("-> None") from all test
function definitions in this file (e.g.
test_propagates_too_many_requests_unchanged,
test_propagates_authentication_error_unchanged,
test_http_401_maps_to_authentication_error,
test_http_429_maps_to_too_many_requests, test_http_404_maps_to_client_error,
test_http_500_maps_to_generic_http_error,
test_existing_connection_error_without_status_still_wrapped,
test_generic_exception_wrapped_as_connection_error,
test_successful_call_returns_value_unchanged) so they follow the tests/**
guideline; update each def signature to drop the "-> None" return annotation
while leaving the decorator `@_handle_api_errors`, parameters, and function body
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b9764f0-609b-4f1f-a7f7-4f67b2e8150a
📒 Files selected for processing (2)
garminconnect/__init__.pytests/test_error_handler_decorator.py
Comment on lines
+36
to
+133
| def test_propagates_too_many_requests_unchanged() -> None: | ||
| """A ``GarminConnectTooManyRequestsError`` raised from inside the wrapped | ||
| function must propagate unchanged — not be wrapped as a generic | ||
| connection error. This is the exact bug from CodeRabbit on PR #352.""" | ||
|
|
||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str) -> None: | ||
| raise GarminConnectTooManyRequestsError("rate limited directly") | ||
|
|
||
| with pytest.raises(GarminConnectTooManyRequestsError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "rate limited directly" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_propagates_authentication_error_unchanged() -> None: | ||
| """A ``GarminConnectAuthenticationError`` raised from inside the wrapped | ||
| function must propagate unchanged.""" | ||
|
|
||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str) -> None: | ||
| raise GarminConnectAuthenticationError("auth failed directly") | ||
|
|
||
| with pytest.raises(GarminConnectAuthenticationError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "auth failed directly" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_http_401_maps_to_authentication_error() -> None: | ||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str) -> None: | ||
| raise _http_error(401, "unauthorized") | ||
|
|
||
| with pytest.raises(GarminConnectAuthenticationError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "Authentication failed" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_http_429_maps_to_too_many_requests() -> None: | ||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str) -> None: | ||
| raise _http_error(429, "slow down") | ||
|
|
||
| with pytest.raises(GarminConnectTooManyRequestsError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "Rate limit exceeded" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_http_404_maps_to_client_error() -> None: | ||
| @_handle_api_errors("Download") | ||
| def inner(self: object, path: str) -> None: | ||
| raise _http_error(404, "nope") | ||
|
|
||
| with pytest.raises(GarminConnectConnectionError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "Download client error (404)" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_http_500_maps_to_generic_http_error() -> None: | ||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str) -> None: | ||
| raise _http_error(500, "server") | ||
|
|
||
| with pytest.raises(GarminConnectConnectionError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "HTTP error" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_existing_connection_error_without_status_still_wrapped() -> None: | ||
| """A plain ``GarminConnectConnectionError`` with no ``.response`` still | ||
| falls through to the HTTP-error wrap branch.""" | ||
|
|
||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str) -> None: | ||
| raise GarminConnectConnectionError("original message") | ||
|
|
||
| with pytest.raises(GarminConnectConnectionError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "original message" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_generic_exception_wrapped_as_connection_error() -> None: | ||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str) -> None: | ||
| raise ValueError("something unexpected") | ||
|
|
||
| with pytest.raises(GarminConnectConnectionError) as excinfo: | ||
| inner(_Dummy(), "/foo") | ||
| assert "Connection error" in str(excinfo.value) | ||
| assert "something unexpected" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_successful_call_returns_value_unchanged() -> None: | ||
| @_handle_api_errors("API call") | ||
| def inner(self: object, path: str, extra: int = 0) -> dict[str, int]: | ||
| return {"path": len(path), "extra": extra} | ||
|
|
||
| result = inner(_Dummy(), "/foo", extra=5) | ||
| assert result == {"path": 4, "extra": 5} |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove return type hints from test functions.
Per coding guidelines for tests/**, test functions shouldn't have a return type hint. All test functions currently include -> None.
♻️ Proposed fix
-def test_propagates_too_many_requests_unchanged() -> None:
+def test_propagates_too_many_requests_unchanged():
"""A ``GarminConnectTooManyRequestsError`` raised from inside the wrapped
...
-def test_propagates_authentication_error_unchanged() -> None:
+def test_propagates_authentication_error_unchanged():
"""A ``GarminConnectAuthenticationError`` raised from inside the wrapped
...
-def test_http_401_maps_to_authentication_error() -> None:
+def test_http_401_maps_to_authentication_error():
`@_handle_api_errors`("API call")
...
-def test_http_429_maps_to_too_many_requests() -> None:
+def test_http_429_maps_to_too_many_requests():
`@_handle_api_errors`("API call")
...
-def test_http_404_maps_to_client_error() -> None:
+def test_http_404_maps_to_client_error():
`@_handle_api_errors`("Download")
...
-def test_http_500_maps_to_generic_http_error() -> None:
+def test_http_500_maps_to_generic_http_error():
`@_handle_api_errors`("API call")
...
-def test_existing_connection_error_without_status_still_wrapped() -> None:
+def test_existing_connection_error_without_status_still_wrapped():
"""A plain ``GarminConnectConnectionError`` with no ``.response`` still
...
-def test_generic_exception_wrapped_as_connection_error() -> None:
+def test_generic_exception_wrapped_as_connection_error():
`@_handle_api_errors`("API call")
...
-def test_successful_call_returns_value_unchanged() -> None:
+def test_successful_call_returns_value_unchanged():
`@_handle_api_errors`("API call")
...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_propagates_too_many_requests_unchanged() -> None: | |
| """A ``GarminConnectTooManyRequestsError`` raised from inside the wrapped | |
| function must propagate unchanged — not be wrapped as a generic | |
| connection error. This is the exact bug from CodeRabbit on PR #352.""" | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise GarminConnectTooManyRequestsError("rate limited directly") | |
| with pytest.raises(GarminConnectTooManyRequestsError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "rate limited directly" in str(excinfo.value) | |
| def test_propagates_authentication_error_unchanged() -> None: | |
| """A ``GarminConnectAuthenticationError`` raised from inside the wrapped | |
| function must propagate unchanged.""" | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise GarminConnectAuthenticationError("auth failed directly") | |
| with pytest.raises(GarminConnectAuthenticationError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "auth failed directly" in str(excinfo.value) | |
| def test_http_401_maps_to_authentication_error() -> None: | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(401, "unauthorized") | |
| with pytest.raises(GarminConnectAuthenticationError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Authentication failed" in str(excinfo.value) | |
| def test_http_429_maps_to_too_many_requests() -> None: | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(429, "slow down") | |
| with pytest.raises(GarminConnectTooManyRequestsError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Rate limit exceeded" in str(excinfo.value) | |
| def test_http_404_maps_to_client_error() -> None: | |
| @_handle_api_errors("Download") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(404, "nope") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Download client error (404)" in str(excinfo.value) | |
| def test_http_500_maps_to_generic_http_error() -> None: | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(500, "server") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "HTTP error" in str(excinfo.value) | |
| def test_existing_connection_error_without_status_still_wrapped() -> None: | |
| """A plain ``GarminConnectConnectionError`` with no ``.response`` still | |
| falls through to the HTTP-error wrap branch.""" | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise GarminConnectConnectionError("original message") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "original message" in str(excinfo.value) | |
| def test_generic_exception_wrapped_as_connection_error() -> None: | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise ValueError("something unexpected") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Connection error" in str(excinfo.value) | |
| assert "something unexpected" in str(excinfo.value) | |
| def test_successful_call_returns_value_unchanged() -> None: | |
| @_handle_api_errors("API call") | |
| def inner(self: object, path: str, extra: int = 0) -> dict[str, int]: | |
| return {"path": len(path), "extra": extra} | |
| result = inner(_Dummy(), "/foo", extra=5) | |
| assert result == {"path": 4, "extra": 5} | |
| def test_propagates_too_many_requests_unchanged(): | |
| """A ``GarminConnectTooManyRequestsError`` raised from inside the wrapped | |
| function must propagate unchanged — not be wrapped as a generic | |
| connection error. This is the exact bug from CodeRabbit on PR `#352`.""" | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise GarminConnectTooManyRequestsError("rate limited directly") | |
| with pytest.raises(GarminConnectTooManyRequestsError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "rate limited directly" in str(excinfo.value) | |
| def test_propagates_authentication_error_unchanged(): | |
| """A ``GarminConnectAuthenticationError`` raised from inside the wrapped | |
| function must propagate unchanged.""" | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise GarminConnectAuthenticationError("auth failed directly") | |
| with pytest.raises(GarminConnectAuthenticationError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "auth failed directly" in str(excinfo.value) | |
| def test_http_401_maps_to_authentication_error(): | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(401, "unauthorized") | |
| with pytest.raises(GarminConnectAuthenticationError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Authentication failed" in str(excinfo.value) | |
| def test_http_429_maps_to_too_many_requests(): | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(429, "slow down") | |
| with pytest.raises(GarminConnectTooManyRequestsError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Rate limit exceeded" in str(excinfo.value) | |
| def test_http_404_maps_to_client_error(): | |
| `@_handle_api_errors`("Download") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(404, "nope") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Download client error (404)" in str(excinfo.value) | |
| def test_http_500_maps_to_generic_http_error(): | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise _http_error(500, "server") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "HTTP error" in str(excinfo.value) | |
| def test_existing_connection_error_without_status_still_wrapped(): | |
| """A plain ``GarminConnectConnectionError`` with no ``.response`` still | |
| falls through to the HTTP-error wrap branch.""" | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise GarminConnectConnectionError("original message") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "original message" in str(excinfo.value) | |
| def test_generic_exception_wrapped_as_connection_error(): | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str) -> None: | |
| raise ValueError("something unexpected") | |
| with pytest.raises(GarminConnectConnectionError) as excinfo: | |
| inner(_Dummy(), "/foo") | |
| assert "Connection error" in str(excinfo.value) | |
| assert "something unexpected" in str(excinfo.value) | |
| def test_successful_call_returns_value_unchanged(): | |
| `@_handle_api_errors`("API call") | |
| def inner(self: object, path: str, extra: int = 0) -> dict[str, int]: | |
| return {"path": len(path), "extra": extra} | |
| result = inner(_Dummy(), "/foo", extra=5) | |
| assert result == {"path": 4, "extra": 5} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_error_handler_decorator.py` around lines 36 - 133, Remove the
return type hints ("-> None") from all test function definitions in this file
(e.g. test_propagates_too_many_requests_unchanged,
test_propagates_authentication_error_unchanged,
test_http_401_maps_to_authentication_error,
test_http_429_maps_to_too_many_requests, test_http_404_maps_to_client_error,
test_http_500_maps_to_generic_http_error,
test_existing_connection_error_without_status_still_wrapped,
test_generic_exception_wrapped_as_connection_error,
test_successful_call_returns_value_unchanged) so they follow the tests/**
guideline; update each def signature to drop the "-> None" return annotation
while leaving the decorator `@_handle_api_errors`, parameters, and function body
unchanged.
Owner
|
@copilot resolve the merge conflicts in this pull request |
Owner
|
I have combined this with your other two PR's which had massive overlap. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Noticed that
connectapi(),connectwebproxy(), anddownload()have ~50 lines of nearly identical error-handling code — each one catchesHTTPError/GarminConnectConnectionError, pulls the status code off the response, logs, and then maps 401 -> auth error, 429 -> rate limit, 4xx -> client error, rest -> HTTP error, plus a genericException-> connection error catch-all.This PR pulls that logic out into a single
_handle_api_errors(label)decorator factory so each of the three methods collapses to a one-liner.Before
After
The decorator itself lives above the
Garminclass and looks like this:Zero behavior change
GarminConnectAuthenticationErrorGarminConnectTooManyRequestsErrorGarminConnectConnectionError("{label}client error")GarminConnectConnectionError("{label}HTTP error")GarminConnectConnectionError("Connection error")logger.exceptionstill fires on every failure path, keyed by the samelabelstrings."Download client error (400): ..."instead of"Download error: ..."on the 4xx branch — which I think is actually more informative — but the exception classes are identical).Net change:
+54 / -78lines ingarminconnect/__init__.py.Why
Beyond just deduping, the main payoff is that any future cross-cutting concern — retries with backoff, structured logging, metrics, circuit breakers — now has exactly one place to live instead of three. E.g. adding retry on 429 becomes a small change inside the decorator instead of three parallel edits.
Test plan
python -m pytest tests/ -v): 19 passed, 2 skipped — the two skips are pre-existing conditional skips insidetest_download_activityandtest_upload.HTTPError(status=401/429/404/500)and a genericValueError— all five branches map to the expected exception types and messages.return_valueflows back through the wrapper unchanged).Summary by CodeRabbit