Skip to content

fix: prevent TogetherException repr crash on non-JSON-serializable headers#431

Open
giulio-leone wants to merge 2 commits intotogethercomputer:mainfrom
giulio-leone:fix/exception-repr-non-serializable
Open

fix: prevent TogetherException repr crash on non-JSON-serializable headers#431
giulio-leone wants to merge 2 commits intotogethercomputer:mainfrom
giulio-leone:fix/exception-repr-non-serializable

Conversation

@giulio-leone
Copy link

@giulio-leone giulio-leone commented Mar 8, 2026

Problem

TogetherException.__repr__ uses json.dumps() to serialize the exception's attributes (including headers) into a JSON string. When headers contains a non-JSON-serializable object like aiohttp's CIMultiDictProxy, this raises a TypeError:

TypeError: Object of type CIMultiDictProxy is not JSON serializable

This makes it impossible to print(), repr(), or log the exception — the debugging tool itself crashes.

Root Cause

json.dumps() in __repr__ has no fallback for non-serializable types:

def __repr__(self) -> str:
    repr_message = json.dumps(
        {
            "response": self._message,
            "status": self.http_status,
            "request_id": self.request_id,
            "headers": self.headers,  # ← CIMultiDictProxy crashes here
        }
    )

Fix

Add default=str to json.dumps() so non-serializable objects fall back to their string representation:

repr_message = json.dumps(
    {...},
    default=str,  # ← fallback for non-serializable objects
)

This is the standard Python pattern for graceful JSON serialization of arbitrary objects.

Tests

Added 7 unit tests covering:

  • Normal dict headers (regression)
  • CIMultiDictProxy-like headers (the reported bug)
  • None headers (default case)
  • String headers
  • Nested non-serializable values
  • Valid output format verification
  • All exception subclasses inherit the fix

Verified Locally

OLD CODE CRASHES: Object of type FakeMultiDictProxy is not JSON serializable
NEW CODE: OK -> {"headers": "<FakeMultiDictProxy({'Content-Type': 'application/json'})>"}

All 7 new tests pass. All 181 existing unit tests continue to pass.

Fixes #108


Note

Low Risk
Low risk: change is confined to exception stringification and adds tests; behavior only affects how errors are represented/logged when headers are non-JSON-serializable.

Overview
Fixes TogetherException.__repr__ to never crash while logging/printing errors when headers contains non-JSON-serializable types (e.g., CIMultiDictProxy).

Adds _json_safe_headers plus json.dumps(..., default=str) to safely stringify/normalize headers, and introduces a focused unit test suite covering dict/None/string headers, CIMultiDictProxy, nested non-serializable values, and subclass behavior.

Written by Cursor Bugbot for commit df6e869. This will update automatically on new commits. Configure here.

@giulio-leone giulio-leone force-pushed the fix/exception-repr-non-serializable branch from 6cd9a96 to b9c7094 Compare March 8, 2026 22:07
@giulio-leone
Copy link
Author

Friendly ping — CI is green, tests pass, ready for review whenever convenient. Happy to address any feedback. Thanks! 🙏

@giulio-leone
Copy link
Author

Friendly ping — this branch has been refreshed on the latest upstream base and all current review feedback has been addressed. It should be ready for review whenever you have a chance. Happy to make any further changes quickly.

@giulio-leone
Copy link
Author

Friendly ping — rebased on latest and ready for review. Happy to address any feedback!

giulio-leone and others added 2 commits March 23, 2026 22:46
…aders

When headers contain non-JSON-serializable objects (e.g. aiohttp's
CIMultiDictProxy), `json.dumps()` in `__repr__` raises TypeError,
making it impossible to print or log the exception.

Add `default=str` to json.dumps so non-serializable objects fall back
to their string representation instead of crashing.

Fixes togethercomputer#108

Signed-off-by: Giulio Leone <6887247+giulio-leone@users.noreply.github.com>
- preserve mapping-like headers as structured JSON in __repr__
- validate the regression with a real CIMultiDictProxy repro

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@giulio-leone giulio-leone force-pushed the fix/exception-repr-non-serializable branch from b9c7094 to df6e869 Compare March 23, 2026 21:52
@giulio-leone
Copy link
Author

Refreshed this branch on the latest main and strengthened the fix a bit further.

What changed in this follow-up:

  • kept mapping-like headers structured in TogetherException.__repr__ instead of falling back to an opaque string when the object is something like CIMultiDictProxy
  • replaced the fake proxy-only regression with a real multidict.CIMultiDictProxy repro so the test now matches the actual async header shape from the reported issue

Validation (double pass):

  • PYTHONPATH="$wt/src" python -m ruff check src/together/error.py tests/unit/test_error_repr.py
  • PYTHONPATH="$wt/src" python -m pytest tests/unit/test_error_repr.py tests/unit/test_client.py tests/unit/test_async_client.py

Runtime proof (double pass):

  • reproduced TogetherException(message="boom", headers=CIMultiDictProxy(...), http_status=500, request_id="req_123")
  • confirmed repr(exc) now renders successfully as structured JSON with headers preserved as an object, e.g. {\"content-type\": \"application/json\"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can not print TogetherException if is contains non-json objects

1 participant