Skip to content

fix(sagemaker): support multi-byte UTF-8 split across streaming event frames - #46887

Closed
aakash1552005 wants to merge 2 commits into
BerriAI:mainfrom
aakash1552005:fix/sagemaker-streaming-utf8-split
Closed

aakash1552005 wants to merge 2 commits into
BerriAI:mainfrom
aakash1552005:fix/sagemaker-streaming-utf8-split

Conversation

@aakash1552005

Copy link
Copy Markdown

TLDR

Problem this solves:

  • Multi-byte UTF-8 characters split across SageMaker event frames fail decoding or get dropped.
  • SageMaker async stream error events (non-200 status) are swallowed, ending prematurely without notice.

How it solves it:

  • Decodes payload bytes via an incremental UTF-8 decoder on AWSEventStreamDecoder instances.
  • Flushes remaining buffer when stream iterator is exhausted, catching truncated sequences.
  • Removes overly broad except Exception in aiter_bytes so non-200 errors propagate matching sync.

User Flow

Before: A caller streaming from a SageMaker endpoint receives partial responses or unhandled decode errors when tokens contain accented or multi-byte characters split across frames.

  1. The caller runs litellm.completion(model="sagemaker/<endpoint>", messages=[{"role": "user", "content": "Reply with café"}], stream=True).
  2. The endpoint delivers "café" with the two bytes of "é" (0xc3, 0xa9) divided across consecutive PayloadPart event frames.
  3. The sync path crashes with UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc3 in position 29: unexpected end of data, or the async path drops both chunks and silently loses the token.
  4. If the endpoint encounters an error mid-stream, the async path swallows the error event and finishes iteration normally.

After: Both sync and async streams cleanly reconstruct split multi-byte UTF-8 sequences and report endpoint errors faithfully.

  1. The caller runs litellm.completion(model="sagemaker/<endpoint>", messages=[{"role": "user", "content": "Reply with café"}], stream=True).
  2. The endpoint delivers "café" with bytes divided across consecutive PayloadPart event frames.
  3. The incremental decoder holds the initial byte until the continuation byte arrives, yielding the full word "café" without error.
  4. If an error frame is returned (e.g. non-200 status code), both sync and async raise a ValueError with the response details.

Relevant issues

Fixes #46876

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/unit/<your_test_file>.py -v.
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • Every automated review on my PR's latest commit has finished, and I have addressed or replied to each of its findings before requesting a maintainer review

Screenshots / Proof of Fix

Before (2dc6aaf)

  1. Run minimal reproducer with split UTF-8 PayloadPart events:
LiteLLM:WARNING: common_utils.py:245 - UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc3 in position 29: unexpected end of data. Attempting to combine with next event.
LiteLLM:WARNING: common_utils.py:245 - UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa9 in position 0: invalid start byte. Attempting to combine with next event.
whole payload decodes to: data: {"token": {"text": "café"}}
sync : raised UnicodeDecodeError - 'utf-8' codec can't decode byte 0xc3 in position 29: unexpected end of data
async: []

After (4b720b5)

  1. Run the same reproducer:
whole payload decodes to: data: {"token": {"text": "café"}}
sync : ['café']
async: ['café']
  1. Unit tests pass:
============================= 21 passed in 1.21s ==============================

… frames (BerriAI#46876)

Use codecs incremental UTF-8 decoder in AWSEventStreamDecoder so partial multi-byte UTF-8 sequences are buffered across PayloadPart frames instead of raising UnicodeDecodeError or dropping events. Also ensure non-200 endpoint errors propagate in async stream to match sync behavior.

Signed-off-by: AAKASH S S <aakash1552005@gmail.com>
@greptile-apps

greptile-apps Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Automatic reviews are disabled for this repository.

@CLAassistant

CLAassistant commented Oct 16, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codspeed

codspeed Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

⚠️ 1 benchmark spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

✅ 32 untouched benchmarks


Comparing aakash1552005:fix/sagemaker-streaming-utf8-split (c5d6b25) with main (f81e657)

Open in CodSpeed

@BrickerP BrickerP left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Direction looks right (incremental UTF-8 decoder + flush on exhaust, and dropping the broad except Exception so non-200 SageMaker events can surface).

CI is red though: Ruff PT012 on tests/unit/llms/sagemaker/test_sagemaker_common_utils.py (~320) — pytest.raises() is wrapping a multi-statement async def + async for block. Tighten that test (e.g. pull the async iteration into a single statement / helper) so lint can go green before merge.

Also note _parse_message_from_event(..., final=) is added but call sites still use the default final=False; the flush path relies on the separate utf8_decoder.decode(b"", final=True) — fine if intentional, just worth a one-line comment so it does not look unfinished.

@codecov

codecov Bot commented Oct 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.60606% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/llms/sagemaker/common_utils.py 60.60% 13 Missing ⚠️

📢 Thoughts on this report? Let us know!

Signed-off-by: AAKASH S S <aakash1552005@gmail.com>
@aakash1552005

Copy link
Copy Markdown
Author

Thanks for the review @BrickerP!

  1. Fixed Ruff PT012 by pulling the generator and async iterator helper outside of the pytest.raises() block in test_aiter_bytes_raises_on_non_200_event.
  2. Added an explanatory comment in _parse_message_from_event documenting that during stream iteration callers pass final=False, with trailing/incomplete multi-byte sequences flushed upon stream exhaustion via self.utf8_decoder.decode(b"", final=True).

Updated in commit c5d6b25.

@github-actions

Copy link
Copy Markdown
Contributor

#46876 was fixed by #47495 on main, so this pull request is closed. Reopen it if something was missed.

@github-actions github-actions Bot closed this Oct 20, 2026
@mateo-berri

Copy link
Copy Markdown
Contributor

Thanks @aakash1552005, #47495 now raises async ModelStreamErrors the way this PR did, and it is merged. Closing this as covered there

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.

[Bug]: SageMaker streaming breaks when a UTF-8 character is split across two PayloadPart events

4 participants