-
Notifications
You must be signed in to change notification settings - Fork 165
[Bug] Worker.create() ignores pre-loaded LoadedDataConverter, always re-loads via require() #1953
Description
What are you really trying to do?
We use a custom PayloadConverter that registers instanceof-based type transformers (e.g. Decimal via superjson). Our project runs under ESM (--import=tsx). We need instanceof checks in the payload converter to work against values created by activity code in the same process.
Describe the bug
Worker.create() unconditionally calls loadDataConverter() on the dataConverter option, which uses require() to load the module at payloadConverterPath. This creates a separate CJS module cache from the ESM-imported application code. As a result, instanceof checks in custom payload converters silently fail -- classes loaded via require() are different objects from the same classes loaded via import.
The Client already handles this correctly by checking isLoadedDataConverter() before calling loadDataConverter():
// @temporalio/client/src/base-client.ts
const dataConverter = options?.dataConverter ?? {};
this.loadedDataConverter = isLoadedDataConverter(dataConverter)
? dataConverter
: loadDataConverter(dataConverter);The Worker does not have this check:
// @temporalio/worker/src/worker-options.ts:1059
loadedDataConverter: loadDataConverter(opts.dataConverter),The failure is completely silent -- instanceof returns false, the custom serializer is skipped, and values fall through to plain JSON serialization, losing their type information. There is no error or warning.
Minimal Reproduction
See the three attached files for a self-contained reproduction using @temporalio/testing. Dependencies: @temporalio/worker, @temporalio/testing, decimal.js.
bug-repro-payload-converter.ts-- a minimal customPayloadConverterthat detectsDecimalvalues viainstanceofand wraps them with format metadata for round-tripping.bug-repro-workflows.ts-- a minimal workflow that calls an activity returning aDecimal, then inspects the result type.bug-repro.test.ts-- two test cases:- "BUG: without workaround" -- demonstrates that
Decimalsilently degrades to a plain string (typeof result === 'string',instanceof Decimal === false, Decimal methods lost). - "WORKAROUND: replacing CJS converter" -- demonstrates that mutating
worker.options.loadedDataConverter.payloadConverterafterWorker.create()fixes the issue.
- "BUG: without workaround" -- demonstrates that
Run with: npx tsx --test bug-repro.test.ts
What happens: The activity creates a Decimal using the ESM module (decimal.mjs). The Worker serializes it using the payload converter loaded via require(), where instanceof Decimal checks against the CJS class (decimal.js). These are different class objects:
import { Decimal as EsmDecimal } from 'decimal.js'; // decimal.mjs
const { Decimal: CjsDecimal } = require('decimal.js'); // decimal.js
EsmDecimal === CjsDecimal; // false
new EsmDecimal('1') instanceof CjsDecimal; // falseSo instanceof returns false, the custom serializer is skipped, and the Decimal is serialized as a plain JSON string. On the receiving side, calling .isZero() on the deserialized value throws isZero is not a function.
Workaround: Mutate the loaded converter after Worker.create():
import { payloadConverter } from './payload-converter';
const worker = await Worker.create({ /* ... */ });
worker.options.loadedDataConverter.payloadConverter = payloadConverter;Environment/Versions
- OS and processor: M1 Mac (also reproducible on x86 Linux)
- SDK version:
@temporalio/worker1.15.0 - Using Temporal Cloud (but also reproducible with the local test server from
@temporalio/testing)
Additional context
Repro Test Files
Suggested fix -- apply the same isLoadedDataConverter guard that the Client already uses:
--- a/packages/worker/src/worker-options.ts
+++ b/packages/worker/src/worker-options.ts
@@ -1,4 +1,4 @@
-import { loadDataConverter } from '@temporalio/common/lib/internal-non-workflow';
+import { isLoadedDataConverter, loadDataConverter } from '@temporalio/common/lib/internal-non-workflow';
@@ -1056,7 +1056,9 @@
defaultHeartbeatThrottleIntervalMs: msToNumber(opts.defaultHeartbeatThrottleInterval),
- loadedDataConverter: loadDataConverter(opts.dataConverter),
+ loadedDataConverter: isLoadedDataConverter(opts.dataConverter)
+ ? opts.dataConverter
+ : loadDataConverter(opts.dataConverter),
activities,This lets users pass a pre-loaded LoadedDataConverter directly, bypassing require() and keeping the same module cache as the activity code:
import { payloadConverter } from './payload-converter';
import { defaultFailureConverter } from '@temporalio/common';
const worker = await Worker.create({
// ...
dataConverter: {
payloadConverter, // ESM-imported instance
failureConverter: defaultFailureConverter,
payloadCodecs: [],
},
});This is especially impactful for packages with separate CJS/ESM entry points (like decimal.js, neverthrow, etc.) where Node.js loads them into separate module caches, causing all instanceof-based type detection to silently break.