Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sure ipc with json serialization still works when bun is parent and not the child #14756

Merged
merged 15 commits into from
Jan 12, 2025

Conversation

nektro
Copy link
Member

@nektro nektro commented Oct 23, 2024

Closes #13799


before

SyntaxError: Unexpected token '', ""I am your father"" is not valid JSON
    at parse (<anonymous>)
    at parseChannelMessages (node:internal/child_process/serialization:152:15)
    at parseChannelMessages.next (<anonymous>)
    at channel.onread (node:internal/child_process:623:18)

after

passes

@robobun
Copy link

robobun commented Oct 23, 2024

Updated 1:29 PM PT - Jan 11th, 2025

@nektro, your commit c4f317a has passed in #9459! 🎉


🧪   try this PR locally:

bunx bun-pr 14756

@nektro nektro marked this pull request as ready for review October 23, 2024 23:47
@nektro nektro requested a review from Jarred-Sumner October 23, 2024 23:52
src/bun.js/ipc.zig Outdated Show resolved Hide resolved
src/bun.js/ipc.zig Outdated Show resolved Hide resolved
src/bun.js/ipc.zig Outdated Show resolved Hide resolved
@nektro nektro requested a review from dylan-conway October 24, 2024 03:15
@nektro nektro enabled auto-merge October 25, 2024 00:38
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner left a comment

Choose a reason for hiding this comment

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

Please add a test that checks node -> bun IPC works as well.

Please also add a test that runs the invalid JSON.parse case matches the behavior of Node when it receives invalid JSON.

src/bun.js/ipc.zig Outdated Show resolved Hide resolved
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner left a comment

Choose a reason for hiding this comment

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

looks like it's printing garbage to stdout on Windows
image

@nektro nektro requested a review from Jarred-Sumner January 12, 2025 03:08
@Jarred-Sumner Jarred-Sumner merged commit 11feeff into main Jan 12, 2025
69 checks passed
@Jarred-Sumner Jarred-Sumner deleted the nektro-patch-63019 branch January 12, 2025 03:18
cirospaciari pushed a commit that referenced this pull request Jan 13, 2025
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.

\x01 byte written to child process
4 participants