fix(traceloop-sdk): avoid calling async json methods in JSONEncoder

[trinhchien]

Summary

• avoid calling async .json() methods during input serialization
• keep workflow tracing intact by falling back to class-name serialization for async-only request-like objects
• add a regression test for async workflows receiving an argument with an async json() method

Why

JSONEncoder currently calls any .json() method it finds synchronously. For objects like Starlette/FastAPI Request, json() is async, which leads to RuntimeWarning: coroutine ... was never awaited during workflow input serialization.

Testing

• pytest packages/traceloop-sdk/tests/test_workflows.py -q -k 'async_json_method_argument'
• pytest packages/traceloop-sdk/tests/test_workflows.py -q -k 'dataclass_serialization_workflow or async_json_method_argument'

Summary by CodeRabbit

• Bug Fixes

  • Improved JSON serialization during tracing to correctly handle objects that expose a JSON method, avoiding accidental awaiting of async callables and preventing incorrect serialization.

• Tests

  • Added synchronous and asynchronous workflow tests to verify argument serialization and trace span input/output recording when workflow arguments provide a JSON method.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Added synchronous and asynchronous tests verifying workflow argument serialization for objects with a json method; updated the JSON encoder to only call an object's json attribute when it is a callable, not an async coroutine function, and when the invocation does not return a coroutine.

Changes

Cohort / File(s)	Summary
Workflow tests packages/traceloop-sdk/tests/test_workflows.py	Added FakeAsyncRequest (async json) and FakeSyncRequest (sync json) and two tests: an async @workflow test asserting a single request_workflow.workflow span with TRACELOOP_ENTITY_INPUT showing args: ["FakeAsyncRequest"] and empty kwargs, and TRACELOOP_ENTITY_OUTPUT {"ok": True}; and a sync @workflow test asserting input args: [{"ok": True}], empty kwargs, and output {"ok": True}.
JSON encoder logic packages/traceloop-sdk/traceloop/sdk/utils/json_encoder.py	Modified JSONEncoder.default to retrieve o.json, ensure it's callable and not an async coroutine function, invoke it, and only return the result if the invocation does not yield a coroutine (avoids unconditionally calling async json).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through tests with eager cheer,
Async and sync friends gather near,
Encoder peeks before it calls,
No dangling coroutine falls,
Tiny paws tidy spans — hooray!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and specifically describes the main change: preventing JSONEncoder from calling async json methods synchronously.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/traceloop-sdk/traceloop/sdk/utils/json_encoder.py`:
- Around line 18-23: The branch handling hasattr(o, "json") currently only
returns a value when o.json is callable; to preserve existing behavior you must
also handle non-callable json attributes: keep the current callable path (use
json_method = o.json, call it when callable and not inspect.iscoroutinefunction,
and return result if not inspect.iscoroutine(result)), and add an else branch
that returns the non-callable o.json value directly (without calling) so it
doesn't fall through to the class-name fallback; ensure these checks use the
same symbols (hasattr(o, "json"), json_method, inspect.iscoroutinefunction,
inspect.iscoroutine) and place this logic before the fallback that returns the
object's class name.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 316c3afc-d5d3-4e32-8875-d4d79cc263d2

📥 Commits

Reviewing files that changed from the base of the PR and between 786d49f and c1a3566.

📒 Files selected for processing (2)

• packages/traceloop-sdk/tests/test_workflows.py
• packages/traceloop-sdk/traceloop/sdk/utils/json_encoder.py

[max-deygin-traceloop]

If possible, please add a regression test for the sync json() method.
This is minor, we can merge without it.

[coderabbitai]

🧹 Nitpick comments (1)

packages/traceloop-sdk/tests/test_workflows.py (1)

511-528: Strengthen regression by asserting no unawaited-coroutine warning.

The current assertions validate serialized values, but the original bug was the RuntimeWarning side effect. Add an explicit warning assertion so this cannot regress silently.

Proposed test hardening

 `@pytest.mark.asyncio`
-async def test_async_workflow_with_async_json_method_argument(exporter):
+async def test_async_workflow_with_async_json_method_argument(exporter, recwarn):
     `@workflow`(name="request_workflow")
     async def request_workflow(request: FakeAsyncRequest):
         return {"ok": True}

     await request_workflow(FakeAsyncRequest())
+    assert not any(
+        w.category is RuntimeWarning and "was never awaited" in str(w.message)
+        for w in recwarn
+    )

     spans = exporter.get_finished_spans()
     assert [span.name for span in spans] == ["request_workflow.workflow"]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/traceloop-sdk/tests/test_workflows.py` around lines 511 - 528, Wrap
the call to await request_workflow(FakeAsyncRequest()) inside a warnings-capture
context and assert no RuntimeWarning was emitted so the regression can't slip
by; specifically modify the test_async_workflow_with_async_json_method_argument
test to capture warnings (e.g., using warnings.catch_warnings(record=True) or
pytest.warns(None) around the await) and then assert that none of the captured
warnings have category RuntimeWarning, keeping the rest of the assertions
(exporter spans, JSON attributes) unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/traceloop-sdk/tests/test_workflows.py`:
- Around line 511-528: Wrap the call to await
request_workflow(FakeAsyncRequest()) inside a warnings-capture context and
assert no RuntimeWarning was emitted so the regression can't slip by;
specifically modify the test_async_workflow_with_async_json_method_argument test
to capture warnings (e.g., using warnings.catch_warnings(record=True) or
pytest.warns(None) around the await) and then assert that none of the captured
warnings have category RuntimeWarning, keeping the rest of the assertions
(exporter spans, JSON attributes) unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f789923d-7462-4c8a-aa33-25620db1c49c

📥 Commits

Reviewing files that changed from the base of the PR and between 625bb89 and d2bf3b2.

📒 Files selected for processing (1)

• packages/traceloop-sdk/tests/test_workflows.py

[max-deygin-traceloop]

thank you @trinhchien, LGTM, if you don't mind please sign the CLA and we can merge

[trinhchien]

thank you @trinhchien, LGTM, if you don't mind please sign the CLA and we can merge

Thanks for the heads-up — I found that the issue was caused by using the wrong commit email on my side. I’ve now rewritten the PR commits with the correct GitHub-linked email and pushed the updated branch. Could you please re-check the CLA status?

[OzBenSimhonTraceloop]

Suggested change

	if not inspect.iscoroutine(result):
	return result
	if not inspect.iscoroutine(result):
	return result
	else:
	result.close()

Also lets verify in tests that no runtime warning is emitted