Add SerializationContext API for core payload/failure conversion paths#1996
Add SerializationContext API for core payload/failure conversion paths#1996
SerializationContext API for core payload/failure conversion paths#1996Conversation
Wire serialization context through the core workflow, worker, and primary client paths so custom payload and failure converters can see which workflow or activity owns each payload. Add focused tests for the main success and failure boundaries. See tests in test-serialization-context.ts and the workflows it uses in test/src/workflows/serialization-context.ts to see the current boundaries where serialization context is implemented.
THardy98
force-pushed
the
feat/ts-serialization-context-core
branch
from
8377564 to
234615e
Compare
THardy98
force-pushed
the
feat/ts-serialization-context-core
branch
from
72d109a to
ab2bd73
Compare
mjameswh
left a comment
mjameswh
left a comment
There was a problem hiding this comment.
At workflow-client.ts:1615 (new code), we pass the unbound data converter to executionInfoFromRaw so that we can later decode a workflow's memo, which is correct.
But then, I'd expect executionInfoFromRaw to bind workflow id when actually calling the data converter.
| import type { PayloadConverter } from './payload-converter'; | ||
|
|
||
| /** | ||
| * Context for payloads owned by a workflow. |
There was a problem hiding this comment.
| * Context for payloads owned by a workflow. | |
| * Context provided to data converters for payloads owned by a workflow. |
| const activator = getActivator(); | ||
| validateActivityOptions(options); | ||
| const activityId = options.activityId ?? `${seq}`; | ||
| const context = activitySerializationContext(activator.info, activityId, false); |
There was a problem hiding this comment.
What's the use case for "activity id" in the activity serialization context?
There was a problem hiding this comment.
standalone activities
There was a problem hiding this comment.
I see. But in the case of activities scheduled from a workflow, I'm not sure passing seq as activity id is desirable. I don't see how users could possibly do good use of that information, yet they will like be some that will misuse it (e.g. by assuming that "activity id" would be globally unique, which seq won't be.
2 participants
What was changed
Added
SerializationContextAPI for core payload/failure conversion paths (i.e. workflows and activities).Scope of this PR is limited to core workflow/activity boundaries (as an effort to split: #1950 into easier to review chunks).
Specifically, the boundaries implemented in this PR are:
isLocal=true)isLocal=true)isLocal=true)Note for reviewers:
The diff looks big but about half of it is the test suite at
test-serialization-context.ts. It might be easier to review this PR starting there, to see what the serialization context across these boundaries look like.Part of [[Feature Request] Serialization context for codecs and converters #1661]
How was this tested:
test-serialization-context.tsfor the boundaries implementedtest-worker-heartbeating.tsto check serialization context is correct whenactivity starttask fails (specifically when it fails duringextractActivityInfofor a failed activity heartbeat, we want to check that the serialization context is propagated through the failure correctly).