Skip to content

Conversation

@jakelazaroff
Copy link
Contributor

@jakelazaroff jakelazaroff commented Mar 10, 2025

What it says on the tin!

if (done) return

const lines = decoder
.decode(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider handling partial JSON lines: use decoder.decode() with { stream: true } and accumulate unfinished lines across chunks to avoid parsing errors.

Suggested change
.decode(value)
.decode(value, { stream: true })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good bot, I didn't know about this and it ended up leading me to TextDecoderStream which is really what we want.

@jakelazaroff
Copy link
Contributor Author

These tests should pass now that https://github.com/jamsocket/forevervm-platform/pull/242 is merged.

@jakelazaroff jakelazaroff merged commit b8f8808 into main Mar 11, 2025
3 checks passed
@jakelazaroff jakelazaroff deleted the jake/exec-result-stream branch March 11, 2025 16:19
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.

2 participants