fix: Raise error for non-JSON-serializable parameters

[echarrod]

Summary

Fixes #69 by providing clear error messages when request parameters cannot be serialized to JSON, instead of silently suppressing the error and returning a confusing API error.

Previously, passing non-JSON-serializable types (like Decimal) would silently fail and set params to None, causing misleading API errors. Now, a clear TypeError is raised immediately pointing to the serialization issue.

Changes

• Modified BaseClient.do() to catch only TypeError instead of suppressing all exceptions
• Raises descriptive error message for non-serializable parameters

Summary by CodeRabbit

• Bug Fixes

  • Clearer error message when request parameters cannot be JSON-serialised.
  • Graceful handling of empty requests so no unnecessary processing is attempted.

• Tests

  • Added tests verifying non-serialisable parameters raise a descriptive error and that empty requests are handled without error.

[echarrod]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

The diff changes BaseClient.do to skip JSON serialization when req is None and to explicitly attempt to JSON-serialize non-None req, raising a descriptive TypeError if serialization fails (previous code swallowed serialization errors). Tests were added to assert that non-JSON-serialisable parameters (Decimal and a custom object) raise a TypeError containing "JSON-serializable", and that a None req proceeds without error and returns the expected response.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

The edits are small and localised: a guard clause and tightened exception handling in one client file, plus straightforward test additions that follow a consistent pattern. Review mainly checks correctness of the guard and error message and that tests properly mock responses.

Poem

🐰 A little guard sits by the JSON door,
No more quiet failures rolling on the floor.
If Decimal stumbles or objects stray,
A helpful TypeError lights the way.
Hoppity cheers — the bug hops away! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "fix: Raise error for non-JSON-serializable parameters" is clear, concise, and directly aligned with the main change in the codebase. The raw summary confirms that the primary modification is adding a guard to explicitly raise a descriptive TypeError when request parameters cannot be JSON-serialised, rather than silently suppressing the error. The title accurately captures this core objective without unnecessary verbosity or vague terminology.
Linked Issues Check	✅ Passed	The pull request directly addresses all coding requirements from issue #69. The modifications to BaseClient.do() implement the core objective by changing error handling to catch only TypeError during JSON serialisation and immediately raise a descriptive error message when parameters are not JSON-serialisable, rather than silently suppressing exceptions and emptying parameters. The new test in test_client.py validates that non-JSON-serialisable types (such as Decimal) raise a TypeError containing "JSON-serialisable" and that None requests are handled appropriately. These changes align precisely with the issue's requirements to surface clear, actionable errors and prevent silent parameter emptying that leads to misleading API responses.
Out of Scope Changes Check	✅ Passed	All changes in this pull request are directly scoped to addressing issue #69 regarding JSON serialisation error handling. The modifications to luno_python/base_client.py focus specifically on improving error handling for non-serialisable parameters, and the additions to tests/test_client.py provide corresponding test coverage for the new behaviour. No unrelated changes, refactoring, or ancillary modifications are present that fall outside the stated objectives of fixing parameter serialisation error handling and providing clear error messages.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ed-69-fix-serialization-error-handling

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4a2c41 and 30cc12f.

📒 Files selected for processing (2)

• luno_python/base_client.py (1 hunks)
• tests/test_client.py (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/test_client.py

🧰 Additional context used

📓 Path-based instructions (1)

luno_python/base_client.py

📄 CodeRabbit inference engine (AGENTS.md)

luno_python/base_client.py: Authenticate requests using HTTP Basic Auth with api_key_id and api_key_secret in the BaseClient.
In BaseClient.do(), parse JSON responses and raise APIError when the API returns an error_code/error.

Files:

• luno_python/base_client.py

🔇 Additional comments (2)

luno_python/base_client.py (2)

73-75: Excellent addition of explicit None handling.

The guard clause clearly handles the case where no parameters are provided, avoiding unnecessary serialization and making the code's intent explicit.

---

76-79: Excellent error handling that fully addresses the PR objectives.

The explicit TypeError handling provides clear, actionable feedback when parameters aren't JSON-serialisable, directly addressing issue #69. The implementation correctly:

• Catches only TypeError (not suppressing all exceptions)
• Raises a descriptive error immediately instead of silently setting params to None
• Preserves the exception chain with from e for full debugging context
• Validates serializability whilst normalising to a plain dict for the requests library

The round-trip serialisation ensures both validation and type normalisation, which is appropriate for this use case.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71f9031 and a4a2c41.

📒 Files selected for processing (2)

• luno_python/base_client.py (1 hunks)
• tests/test_client.py (2 hunks)

🧰 Additional context used

🪛 Ruff (0.14.1)

luno_python/base_client.py

79-79: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🔇 Additional comments (2)

tests/test_client.py (2)

4-4: LGTM!

Good addition of the Decimal import to support testing non-serialisable parameters.

---

225-248: Excellent test coverage for the new error handling.

This test thoroughly validates the fix by covering:

• Non-serialisable types that commonly cause issues (Decimal)
• Custom objects
• The None case to ensure it still works correctly

The assertions properly verify that the error messages contain 'JSON-serializable', ensuring developers receive actionable feedback.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud