Skip to content

Conversation

@yuandrew
Copy link
Contributor

@yuandrew yuandrew commented Dec 1, 2025

What was changed

Introduced an SDK flag, SDKFlagMemoUserDCEncode, to configure new behavior, where we actually use the user data converter when encoding Memo's. If user DC errors out, the default DC will be used as a fallback. If that also returns an error, the user DC error will be returned to the caller.

SDK flag is still set to false by default, so no behavior changes. After a Go SDK release or two, will the flag then be enabled by default.

Why?

Correct behavior to match other SDKs

Checklist

  1. Closes Memo does not go through user provided data coverter #1045

  2. How was this tested:
    Added new tests that flip on/off the SDK flag and validates behavior

  3. Any docs updates needed?


Note

Adds SDKFlagMemoUserDCEncode to optionally encode workflow/schedule memos with the user data converter (falling back to default), and updates codepaths and tests to honor it.

  • Flags & config:
    • Introduce SDKFlagMemoUserDCEncode with env toggle MEMO_USER_DC_ENCODE and setter SetMemoUserDCEncode.
  • Memo encoding behavior:
    • Add encodeMemoValue and shouldUseMemoUserDataConverter; prefer user DataConverter for memo encoding, fallback to default on error.
    • Extend getWorkflowMemo(..) and validateAndSerializeMemo(..) to accept a memoFlagAccessor and apply flag.
    • Update all call sites (StartWorkflow, SignalWithStart, child workflow start, schedules, testsuite) to pass accessor and/or use new helpers.
  • Schedule client:
    • encodeScheduleWorkflowMemo now uses user DC (with fallback) and defaults DC when nil.
  • Tests:
    • Add tests validating memo encoding with old/new behavior and failure paths for both workflow and schedule clients.
    • Adjust testsuite memo paths to use new accessor; minor assertions tightened.

Written by Cursor Bugbot for commit 049d0fb. This will update automatically on new commits. Configure here.

@yuandrew yuandrew requested a review from a team as a code owner December 1, 2025 21:35

// memoUserDCEncode exists to allow us to configure the default behavior of
// SDKFlagMemoUserDCEncode. This is primarily useful with tests.
var memoUserDCEncode = os.Getenv("MEMO_USER_DC_ENCODE") != ""
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment says this is useful for tests, but I don't see this used in the SDK anywhere. Why did we need to add this?

Copy link
Contributor Author

@yuandrew yuandrew Dec 9, 2025

Choose a reason for hiding this comment

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

It's set and used in SetMemoUserDCEncode(), so we can defer setting it back to its previous flag

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.

Memo does not go through user provided data coverter

2 participants