sdk-typescript icon indicating copy to clipboard operation
sdk-typescript copied to clipboard

[Bug] Error not serialized fully when sent to Sinks

Open marcoreni opened this issue 1 year ago • 1 comments

What are you really trying to do?

Sending an error to a Sink function and preserve all the attributes.

Describe the bug

It seems that the error that the error received from the sink function has less fields than the error which was sent.

For example: we're creating an ApplicationFailure.nonRetryable from a child workflow, and we're intercepting the error inside the parent workflow.

Error inside the parent workflow:

{
  cause: {
    cause: undefined,
    type: 'my_error_type',
    nonRetryable: true,
    details: [],
    failure: {
      message: 'my_error_message',
      source: 'TypeScriptSDK',
      stackTrace: 'ApplicationFailure: my_error_message\n' +
        '    at Function.nonRetryable (/usr/app/node_modules/.pnpm/@[email protected]/node_modules/@temporalio/common/src/failure.ts:149:11)\n' +
        '    at nonRetryable (/usr/app/src/workflows/index.ts:51:28)',
      applicationFailureInfo: [Object]
    },
    name: 'ApplicationFailure',
    message: 'my_error_message',
    stack: 'ApplicationFailure: my_error_message\n' +
      '    at Function.nonRetryable (/usr/app/node_modules/.pnpm/@[email protected]/node_modules/@temporalio/common/src/failure.ts:149:11)\n' +
      '    at nonRetryable (/usr/app/src/workflows/index.ts:51:28)'
  },
  namespace: 'default',
  execution: {
    workflowId: '1698670214662-foobar',
    runId: 'd46839c0-2255-489b-b8ea-37b41b8501e9'
  },
  workflowType: 'my_workflow_type',
  retryState: 5,
  failure: {
    message: 'Child Workflow execution failed',
    cause: {
      message: 'my_error:message',
      source: 'TypeScriptSDK',
      stackTrace: 'ApplicationFailure: my_error_message\n' +
        '    at Function.nonRetryable (/usr/app/node_modules/.pnpm/@[email protected]/node_modules/@temporalio/common/src/failure.ts:149:11)\n' +
        '    at nonRetryable (/usr/app/src/workflows/index.ts:51:28)',
      applicationFailureInfo: [Object]
    },
    childWorkflowExecutionFailureInfo: {
      namespace: 'default',
      workflowExecution: [Object],
      workflowType: [Object],
      initiatedEventId: '23',
      startedEventId: '24',
      retryState: 'RETRY_STATE_RETRY_POLICY_NOT_SET'
    }
  },
  name: 'ChildWorkflowFailure',
  message: 'Child Workflow execution failed',
  stack: ''
}

Error that is received from the sink:

{
  name: "Error",
  message: "Child Workflow execution failed",
  stack: "",
  cause: {
    name: "Error",
    message: "my_error_message",
    stack:
      "ApplicationFailure: my_error_message\n" +
      "    at Function.nonRetryable (/usr/app/node_modules/.pnpm/@[email protected]/node_modules/@temporalio/common/src/failure.ts:149:11)\n" +
      "    at nonRetryable (/usr/app/src/workflows/index.ts:51:28)",
  },
};

Given that, IIUC, the sink is implemented via a postMessage, would it be possible to extend the serialization format of the error so that more fields are passed through (more specifically, we're currently interested in type and details of the cause)?

Minimal Reproduction

N/A

Environment/Versions

  • OS and processor: M1 Mac
  • Temporal Version: 1.8.6
  • Are you using Docker or Kubernetes or building Temporal from source? Docker

Additional context

marcoreni avatar Oct 30 '23 13:10 marcoreni

This is a known limitation of sinks. As you pointed out, the data is transferred over postMessage from a worker thread to the main Node.js thread which uses the structured clone algorithm.

https://nodejs.org/api/worker_threads.html#portpostmessagevalue-transferlist

Looks like our documentation doesn't call out this limitation clearly enough though. I think we should add this information on proxySinks and the WorkerOptions.sinks.

https://typescript.temporal.io/api/namespaces/workflow#sinkfunction

bergundy avatar Oct 30 '23 19:10 bergundy