Skip to content

Conversation

@bobbytables
Copy link
Contributor

What was changed

Mark the JSON value as binary before assigning to data key in Api::Common::V1::Payload.

Why?

Ruby protobuf uses ASCII-8BIT encoding for bytes fields in proto messages. The protobuf gem will attend to force the encoding from UTF8 to ASCII-8BIT when assigning the value, but when UTF8 has multi-byte characters (like an emdash), it fails.

LLM's love the emdash, so when I was attempting to switch from coinbase's library to this one, I hit issues with the encoding problem. I had commented on this PR on Coinbase's PR which had the same exact problem: coinbase/temporal-ruby#264

I added tests to demonstrate the problem, if you remove the .b in the changes you'll see them fail.

@bobbytables bobbytables requested a review from a team as a code owner October 2, 2025 18:26
@CLAassistant
Copy link

CLAassistant commented Oct 2, 2025

CLA assistant check
All committers have signed the CLA.

@cretz
Copy link
Member

cretz commented Oct 2, 2025

Thanks! I guess we did this with regular JSON.generate and had just assumed proto implicitly used .b not .encode. Will merge if/when CI succeeds.

@cretz cretz merged commit 7354b08 into temporalio:main Oct 2, 2025
10 of 11 checks passed
@bobbytables
Copy link
Contributor Author

Happy to help, thanks for the fast merge.

@bobbytables bobbytables deleted the force-binary-for-protobuf-encoding branch October 3, 2025 19:33
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.

3 participants