-
Notifications
You must be signed in to change notification settings - Fork 162
Standardize Nexus Operation Input Arg. Deserialization Failure #1949
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
Changes from all commits
372d717
55a167c
3c87225
ddddc07
608ad70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { defaultPayloadConverter, Payload } from '@temporalio/common'; | ||
| import { PayloadConverter } from '@temporalio/common/lib/converter/payload-converter'; | ||
|
|
||
| export const payloadConverter: PayloadConverter = { | ||
| toPayload<T>(value: T): Payload { | ||
| return defaultPayloadConverter.toPayload(value); | ||
| }, | ||
| fromPayload<T>(_payload: Payload): T { | ||
| throw new Error('Intentional payload converter failure for testing'); | ||
| }, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| import { randomUUID } from 'crypto'; | ||
| import * as nexus from 'nexus-rpc'; | ||
| import { NexusOperationFailure, Payload } from '@temporalio/common'; | ||
| import { Client, WorkflowFailedError } from '@temporalio/client'; | ||
| import type { PayloadCodec } from '@temporalio/common/lib/converter/payload-codec'; | ||
| import * as workflow from '@temporalio/workflow'; | ||
| import { helpers, makeTestFunction } from './helpers-integration'; | ||
|
|
||
| const test = makeTestFunction({ | ||
| workflowsPath: __filename, | ||
| workflowInterceptorModules: [__filename], | ||
| }); | ||
|
|
||
| const testService = nexus.service('codec-converter-test', { | ||
| echoOp: nexus.operation<string, string>(), | ||
| }); | ||
|
|
||
| export async function nexusEchoCaller(endpoint: string): Promise<string> { | ||
| const client = workflow.createNexusClient({ | ||
| endpoint, | ||
| service: testService, | ||
| }); | ||
| const handle = await client.startOperation('echoOp', 'hello'); | ||
| return await handle.result(); | ||
| } | ||
|
|
||
| //////////////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| test('Nexus operation codec failure is retried', async (t) => { | ||
| const { createWorker, registerNexusEndpoint, taskQueue } = helpers(t); | ||
| const { endpointName } = await registerNexusEndpoint(); | ||
|
|
||
| let decodeCount = 0; | ||
| const failingCodec: PayloadCodec = { | ||
| async encode(payloads: Payload[]): Promise<Payload[]> { | ||
| return payloads; | ||
| }, | ||
| async decode(payloads: Payload[]): Promise<Payload[]> { | ||
| decodeCount++; | ||
| if (decodeCount === 1) { | ||
| throw new Error('Intentional codec decode failure'); | ||
| } | ||
| return payloads; | ||
| }, | ||
| }; | ||
|
|
||
| const worker = await createWorker({ | ||
| dataConverter: { payloadCodecs: [failingCodec] }, | ||
| nexusServices: [ | ||
| nexus.serviceHandler(testService, { | ||
| async echoOp(_ctx, input) { | ||
| return input; | ||
| }, | ||
| }), | ||
| ], | ||
| }); | ||
|
|
||
| const customClient = new Client({ | ||
| connection: t.context.env.connection, | ||
| dataConverter: { payloadCodecs: [failingCodec] }, | ||
| }); | ||
|
|
||
| await worker.runUntil(async () => { | ||
| const result = await customClient.workflow.execute(nexusEchoCaller, { | ||
| taskQueue, | ||
| workflowId: randomUUID(), | ||
| args: [endpointName], | ||
| }); | ||
| t.is(result, 'hello'); | ||
| }); | ||
|
|
||
| t.true(decodeCount >= 2, `Expected decode count >= 2, got ${decodeCount}`); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Codec retry test may hit wrong decodeLow Severity
|
||
|
|
||
| //////////////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| test('Nexus operation converter failure is not retried', async (t) => { | ||
| const { createWorker, registerNexusEndpoint, taskQueue } = helpers(t); | ||
| const { endpointName } = await registerNexusEndpoint(); | ||
|
|
||
| const worker = await createWorker({ | ||
| dataConverter: { payloadConverterPath: require.resolve('./failing-payload-converter') }, | ||
| nexusServices: [ | ||
| nexus.serviceHandler(testService, { | ||
| async echoOp(_ctx, input) { | ||
| return input; | ||
| }, | ||
| }), | ||
| ], | ||
| }); | ||
|
|
||
| await worker.runUntil(async () => { | ||
| const err = await t.throwsAsync( | ||
| () => | ||
| t.context.env.client.workflow.execute(nexusEchoCaller, { | ||
| taskQueue, | ||
| workflowId: randomUUID(), | ||
| args: [endpointName], | ||
| }), | ||
| { | ||
| instanceOf: WorkflowFailedError, | ||
| } | ||
| ); | ||
| t.true(err instanceof WorkflowFailedError); | ||
| t.true(err!.cause instanceof NexusOperationFailure); | ||
| const nexusFailure = err!.cause as NexusOperationFailure; | ||
| t.true(nexusFailure.cause instanceof nexus.HandlerError); | ||
| const handlerError = nexusFailure.cause as nexus.HandlerError; | ||
| t.is(handlerError.type, 'BAD_REQUEST'); | ||
| t.false(handlerError.retryable); | ||
| t.regex(handlerError.message, /Intentional payload converter failure for testing/); | ||
| }); | ||
| }); | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This injection throws on the first
decodecall globally, but the same codec is also configured on the worker (dataConverter.payloadCodecs) where it is used for workflow activation decoding (WorkflowCodecRunner.decodeActivation) before the Nexus handler runs. That means the initial failure can be consumed outside the Nexus operation path, anddecodeCount >= 2can still pass even if Nexus input deserialization is never retried, so this test can miss regressions in the behavior this commit is trying to validate.Useful? React with 👍 / 👎.