Repository navigation
fix(sdk-python,sdk-typescript): keep stream markers that end a log WebSocket message - #352
Open
TanishGudise wants to merge 1 commit into
Open
TanishGudise wants to merge 1 commit into
TanishGudise wants to merge 1 commit into
Conversation
…bSocket message The stdout/stderr log demultiplexer holds back up to two trailing 0x01/0x02 bytes in case a marker is split across messages, but it also only searched for complete markers inside the bytes it did not hold back. When a message ended exactly on a complete marker, the marker was missed: its first byte was emitted as payload, the other two were glued onto the next message, and the stream type never switched. Callers saw raw marker bytes in their output, stderr delivered to on_stdout, or output dropped entirely when no marker had been recognized yet. Search the whole buffer for complete markers; the held-back length now only bounds how much marker-free payload may be flushed. This matches the Go and Java SDKs. Covers the sync and async Python SDKs (shared _std_demux_loop) and the TypeScript stdDemuxStream. Fixes daytona#350 Signed-off-by: Tanish Gudise <174986239+TanishGudise@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Vidoc security reviewTip Good to merge — no security issues found. Reviewed 4 changed files. 💬 Have questions? Tag @vidoc in a comment and I'll answer. |
Contributor
|
All contributors have signed the CLA. ✅ Thank you! |
Author
|
I have read the CLA Document and I hereby sign the CLA |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Streamed command and entrypoint logs could lose output, deliver stderr to the stdout callback, or leak raw
\x01/\x02marker bytes when a WebSocket message ended exactly on a complete 3-byte stream marker. This affected the demultiplexer shared by the sync and async Python SDKs (_std_demux_loop) and the TypeScript SDK'sstdDemuxStream.The demultiplexer holds back up to two trailing marker bytes in case a marker is split across messages, but it also searched for complete markers only in the bytes it had not held back, so a marker at the very end of a message was missed. With this change it searches the whole buffer for complete markers; the held-back length now only limits how much marker-free output is flushed. The Go and Java SDKs already behave this way. The fix is 2 lines in each SDK.
Tests (local only, nothing talks to the Daytona API):
sdk-python/tests/test_stream.py:_std_demux_loopcases: single message; marker split 2+1 and 1+2; a message that is only a marker; a message ending with a stderr marker; a message ending with a stdout markerAsyncProcessand syncProcessget_session_command_logs_asyncagainst a local aiohttp WebSocket server that sends marker-aligned messagessdk-typescript/src/__tests__/Stream.server.test.ts: the same cases throughstdDemuxStreamagainst a real localwsserver, in the style ofConnectionRetry.server.test.tsOn
main, 5 of the 8 new Python tests and 3 of the 6 new TypeScript tests fail; the split-marker and single-message controls pass. With this change, all of them pass.Documentation
No public API or docstring changes.
Related Issue(s)
Fixes #350
Screenshots
N/A
Notes
yarn lint(lint:ts and lint:py: 0 errors) and the pre-commit lint-staged hooksnx run sdk-typescript:test: 21 suites, 450 teststest_event_subscription_manager.py::TestSyncEventSubscriptionManagerThreadUsage::test_many_subscriptions_share_one_expiry_threadfails intermittently onmaintoo (a thread-count assertion; 2 of 8 local runs). It's unrelated to this change.🤖 Generated with Claude Code
Summary by cubic
Fixes the log demultiplexer in the Python and TypeScript SDKs so streamed command and entrypoint logs no longer lose output, misroute stderr, or leak raw marker bytes when a WebSocket message ends exactly on a complete stream marker. The demultiplexer held back up to two trailing marker bytes to handle markers split across messages, but only searched for complete markers in the bytes it did not hold back, so a marker at the very end of a message was missed. Now it searches the whole buffer for complete markers; the held-back length only bounds how much marker-free output is flushed, matching the Go and Java SDKs. Added regression tests for the sync and async Python SDKs and the TypeScript SDK against a real WebSocket server. Fixes #350.
Written for commit 36552b2. Summary will update on new commits.