Skip to content

fix: add payload to exception to improve debugging capabilities #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danieljrc888 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: add payload to exception to improve debugging capabilities #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23590c6
fix: add payload to genlayer exception and raise with response
danieljrc888 Sep 26, 2025
c1d287a
style: apply black
danieljrc888 Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions genlayer_py/consensus/abi/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
import json
import importlib.resources

with importlib.resources.as_file(
importlib.resources.files("genlayer_py.consensus.abi").joinpath(
"consensus_data_abi.json"
)
) as path, open(path, "r", encoding="utf-8") as f:
with (
importlib.resources.as_file(
importlib.resources.files("genlayer_py.consensus.abi").joinpath(
"consensus_data_abi.json"
)
) as path,
open(path, "r", encoding="utf-8") as f,
):
CONSENSUS_DATA_ABI = json.load(f)

with importlib.resources.as_file(
importlib.resources.files("genlayer_py.consensus.abi").joinpath(
"consensus_main_abi.json"
)
) as path, open(path, "r", encoding="utf-8") as f:
with (
importlib.resources.as_file(
importlib.resources.files("genlayer_py.consensus.abi").joinpath(
"consensus_main_abi.json"
)
) as path,
open(path, "r", encoding="utf-8") as f,
):
Comment on lines +4 to +21
Copy link

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Avoid Python 3.10-only with (...) syntax unless you’ve dropped 3.9 support.

Using parentheses around multiple context managers (Line 4) is only valid starting in Python 3.10. If the package still supports Python 3.9 (or earlier), this will raise a SyntaxError at import time and break every consumer. Please stick to the nested with form or otherwise confirm that requires-python has been bumped to >=3.10 across the project before landing this.

🤖 Prompt for AI Agents 
In genlayer_py/consensus/abi/__init__.py around lines 4 to 21, the code uses the
Python 3.10-only "with (a, b):" multiple-context-manager syntax which will raise
SyntaxError on Python 3.9; change these to nested with statements (open the
resource with importlib.resources.as_file(...) as path: then inside that block
open(path, "r", encoding="utf-8") as f:) for each file load, or alternatively
ensure the project's requires-python metadata has been raised to >=3.10 before
keeping the current syntax.

CONSENSUS_MAIN_ABI = json.load(f)

__all__ = ["CONSENSUS_DATA_ABI", "CONSENSUS_MAIN_ABI"]
27 changes: 25 additions & 2 deletions genlayer_py/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,27 @@
from typing import Any, Optional


class GenLayerError(Exception):
"""Base exception class for GenLayer SDK errors.

This exception can carry additional context information through an optional
payload dictionary, useful for debugging and error handling.

Args:
message: Human-readable error description.
payload: Optional dictionary containing additional error context,
such as transaction details, error codes, or debug information.

Attributes:
payload: Dictionary of additional error context (None if not provided).

Example:
>>> raise GenLayerError(
... "Transaction failed",
... {"tx_hash": "0x123...", "reason": "insufficient funds"}
... )
"""
An error raised by GenLayer.
"""

def __init__(self, message: str, payload: Optional[dict[str, Any]] = None):
super().__init__(message)
self.payload = payload
9 changes: 7 additions & 2 deletions genlayer_py/provider/provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,14 @@ def make_request(
try:
resp = response.json()
except ValueError as err:
response_preview = response.text[:500] if len(response.text) <= 500 else f"{response.text[:500]}..."
response_preview = (
response.text[:500]
if len(response.text) <= 500
else f"{response.text[:500]}..."
)
raise GenLayerError(
f"{method} returned invalid JSON: {err}. Response content: {response_preview}"
f"{method} returned invalid JSON: {err}. Response content: {response_preview}",
payload={"response": response},
) from err
Comment on lines +41 to 49
Copy link

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid storing the raw Response object in payload

Keeping the requests.Response instance in GenLayerError.payload makes the payload non‑serializable and exposes request headers (often containing credentials) when downstream tooling logs or forwards the payload. That breaks observability pipelines and increases leakage risk. Please persist only primitive, serializable fields (status code, headers copy, truncated body) so callers can safely inspect the data.

         except ValueError as err:
             response_preview = (
                 response.text[:500]
                 if len(response.text) <= 500
                 else f"{response.text[:500]}..."
             )
+            response_payload = {
+                "status_code": response.status_code,
+                "headers": dict(response.headers),
+                "body_preview": response_preview,
+            }
             raise GenLayerError(
                 f"{method} returned invalid JSON: {err}. Response content: {response_preview}",
-                payload={"response": response},
+                payload=response_payload,
             ) from err
📝 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
response_preview = (
response.text[:500]
if len(response.text) <= 500
else f"{response.text[:500]}..."
)
raise GenLayerError(
f"{method} returned invalid JSON: {err}. Response content: {response_preview}"
f"{method} returned invalid JSON: {err}. Response content: {response_preview}",
payload={"response": response},
) from err
except ValueError as err:
response_preview = (
response.text[:500]
if len(response.text) <= 500
else f"{response.text[:500]}..."
)
response_payload = {
"status_code": response.status_code,
"headers": dict(response.headers),
"body_preview": response_preview,
}
raise GenLayerError(
f"{method} returned invalid JSON: {err}. Response content: {response_preview}",
payload=response_payload,
) from err
🧰 Tools
🪛 Ruff (0.13.1)

46-49: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents 
In genlayer_py/provider/provider.py around lines 41 to 49, the GenLayerError
currently stores the raw requests.Response in payload which is non-serializable
and can leak sensitive headers; instead build a serializable payload with
primitive fields only — e.g., include response.status_code,
dict(response.headers) (a shallow copy), a truncated body (use the existing
response_preview), and optionally response.url or reason — and pass that dict as
payload to GenLayerError; remove the raw Response object from the payload.

self._raise_on_error(resp, method)
return resp
Expand Down
7 changes: 5 additions & 2 deletions genlayer_py/types/transactions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,14 @@ class TransactionStatus(str, Enum):
TransactionStatus.LEADER_TIMEOUT,
TransactionStatus.VALIDATORS_TIMEOUT,
TransactionStatus.CANCELED,
TransactionStatus.FINALIZED
TransactionStatus.FINALIZED,
]


def is_decided_state(status: str) -> bool:
return status in [TRANSACTION_STATUS_NAME_TO_NUMBER[state] for state in DECIDED_STATES]
return status in [
TRANSACTION_STATUS_NAME_TO_NUMBER[state] for state in DECIDED_STATES
]


class TransactionResult(str, Enum):
Expand Down
65 changes: 47 additions & 18 deletions tests/unit/transactions/test_wait_for_transaction_receipt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,15 @@ def test_wait_for_transaction_with_custom_parameters(

def test_wait_for_accepted_with_all_decided_states(self, mock_client):
"""Test that ACCEPTED status accepts all decided states"""
decided_statuses = ["5", "6", "8", "7", "12", "13"] # ACCEPTED, UNDETERMINED, CANCELED, FINALIZED, VALIDATORS_TIMEOUT, LEADER_TIMEOUT

decided_statuses = [
"5",
"6",
"8",
"7",
"12",
"13",
] # ACCEPTED, UNDETERMINED, CANCELED, FINALIZED, VALIDATORS_TIMEOUT, LEADER_TIMEOUT

for status_num in decided_statuses:
mock_transaction = {
"hash": "0x4b8037744adab7ea8335b4f839979d20031d83a8ccdf706e0ae61312930335f6",
Expand All @@ -150,16 +157,16 @@ def test_wait_for_accepted_with_all_decided_states(self, mock_client):
"nonce": "1",
"created_at": "2023-01-01T00:00:00Z",
}

mock_client.get_transaction.return_value = mock_transaction

result = wait_for_transaction_receipt(
self=mock_client,
transaction_hash="0x4b8037744adab7ea8335b4f839979d20031d83a8ccdf706e0ae61312930335f6",
status=TransactionStatus.ACCEPTED,
full_transaction=True,
)

assert result == mock_transaction

def test_wait_for_specific_status_not_affected(self, mock_client):
Expand All @@ -175,16 +182,16 @@ def test_wait_for_specific_status_not_affected(self, mock_client):
"nonce": "1",
"created_at": "2023-01-01T00:00:00Z",
}

mock_client.get_transaction.return_value = mock_transaction

result = wait_for_transaction_receipt(
self=mock_client,
transaction_hash="0x4b8037744adab7ea8335b4f839979d20031d83a8ccdf706e0ae61312930335f6",
status=TransactionStatus.FINALIZED,
full_transaction=True,
)

assert result == mock_transaction


Expand All @@ -199,32 +206,54 @@ def test_decided_states_constant(self):
TransactionStatus.LEADER_TIMEOUT,
TransactionStatus.VALIDATORS_TIMEOUT,
TransactionStatus.CANCELED,
TransactionStatus.FINALIZED
TransactionStatus.FINALIZED,
]

assert DECIDED_STATES == expected_states

def test_is_decided_state_with_decided_statuses(self):
"""Test is_decided_state returns True for all decided statuses"""
decided_status_numbers = ["5", "6", "8", "7", "12", "13"] # ACCEPTED, UNDETERMINED, CANCELED, FINALIZED, VALIDATORS_TIMEOUT, LEADER_TIMEOUT

decided_status_numbers = [
"5",
"6",
"8",
"7",
"12",
"13",
] # ACCEPTED, UNDETERMINED, CANCELED, FINALIZED, VALIDATORS_TIMEOUT, LEADER_TIMEOUT

for status_num in decided_status_numbers:
assert is_decided_state(status_num) == True, f"Status {status_num} should be decided"
assert (
is_decided_state(status_num) == True
), f"Status {status_num} should be decided"

def test_is_decided_state_with_non_decided_statuses(self):
"""Test is_decided_state returns False for non-decided statuses"""
non_decided_status_numbers = ["0", "1", "2", "3", "4", "9", "10", "11"] # UNINITIALIZED, PENDING, PROPOSING, COMMITTING, REVEALING, APPEAL_REVEALING, APPEAL_COMMITTING, READY_TO_FINALIZE

non_decided_status_numbers = [
"0",
"1",
"2",
"3",
"4",
"9",
"10",
"11",
] # UNINITIALIZED, PENDING, PROPOSING, COMMITTING, REVEALING, APPEAL_REVEALING, APPEAL_COMMITTING, READY_TO_FINALIZE

for status_num in non_decided_status_numbers:
assert is_decided_state(status_num) == False, f"Status {status_num} should not be decided"
assert (
is_decided_state(status_num) == False
), f"Status {status_num} should not be decided"

def test_is_decided_state_with_invalid_status(self):
"""Test is_decided_state returns False for invalid statuses"""
invalid_statuses = ["999", "invalid", "", None]

for status in invalid_statuses:
if status is not None:
assert is_decided_state(status) == False, f"Invalid status {status} should not be decided"
assert (
is_decided_state(status) == False
), f"Invalid status {status} should not be decided"


class TestSimplifyTransactionReceipt:
Expand Down