fix(common): Improve error messages for failureConverters (#1373)

Author: antlai-temporal

packages/common/src/internal-non-workflow/data-converter-helpers.ts

@@ -4,19 +4,37 @@ import { FailureConverter } from '../converter/failure-converter';
 import { errorCode, hasOwnProperty, isRecord } from '../type-helpers';
 import { ValueError } from '../errors';
 
-const isValidPayloadConverter = (converter: unknown): converter is PayloadConverter =>
-  typeof converter === 'object' &&
-  converter !== null &&
-  ['toPayload', 'fromPayload'].every((method) => typeof (converter as Record<string, unknown>)[method] === 'function');
+const isValidPayloadConverter = (converter: unknown, path: string): asserts converter is PayloadConverter => {
+  const isValid =
+    typeof converter === 'object' &&
+    converter !== null &&
+    ['toPayload', 'fromPayload'].every(
+      (method) => typeof (converter as Record<string, unknown>)[method] === 'function'
+    );
+  if (!isValid) {
+    throw new ValueError(`payloadConverter export at ${path} must be an object with toPayload and fromPayload methods`);
+  }
+};
 
-const isValidFailureConverter = (converter: unknown): converter is FailureConverter =>
-  typeof converter === 'object' &&
-  converter !== null &&
-  ['errorToFailure', 'failureToError'].every(
-    (method) => typeof (converter as Record<string, unknown>)[method] === 'function'
-  );
+const isValidFailureConverter = (converter: unknown, path: string): asserts converter is FailureConverter => {
+  const isValid =
+    typeof converter === 'object' &&
+    converter !== null &&
+    ['errorToFailure', 'failureToError'].every(
+      (method) => typeof (converter as Record<string, unknown>)[method] === 'function'
+    );
+  if (!isValid) {
+    throw new ValueError(
+      `failureConverter export at ${path} must be an object with errorToFailure and failureToError methods`
+    );
+  }
+};
 
-function requireConverter<T>(path: string, type: string, validator: (converter: unknown) => converter is T): T {
+function requireConverter<T>(
+  path: string,
+  type: string,
+  validator: (converter: unknown, path: string) => asserts converter is T
+): T {
   let module;
   try {
     module = require(path); // eslint-disable-line @typescript-eslint/no-var-requires
@@ -29,15 +47,10 @@ function requireConverter<T>(path: string, type: string, validator: (converter:
 
   if (isRecord(module) && hasOwnProperty(module, type)) {
     const converter = module[type];
-    if (validator(converter)) {
-      return converter;
-    } else {
-      throw new ValueError(
-        `payloadConverter export at ${path} must be an object with toPayload and fromPayload methods`
-      );
-    }
+    validator(converter, path);
+    return converter;
   } else {
-    throw new ValueError(`Module ${path} does not have a \`payloadConverter\` named export`);
+    throw new ValueError(`Module ${path} does not have a \`${type}\` named export`);
   }
 }
 

packages/test/src/payload-converters/failure-converter-bad-export.ts

@@ -0,0 +1 @@
+export const failureConverter = { toPayload: Function.prototype };

packages/test/src/test-custom-payload-converter.ts

@@ -106,6 +106,28 @@ test('Worker throws on invalid payloadConverterPath', async (t) => {
       message: /payloadConverter export at .* must be an object with toPayload and fromPayload methods/,
     }
   );
+
+  t.throws(
+    () =>
+      isolateFreeWorker({
+        ...defaultOptions,
+        dataConverter: { failureConverterPath: require.resolve('./payload-converters/payload-converter-bad-export') },
+      }),
+    {
+      message: /Module .* does not have a `failureConverter` named export/,
+    }
+  );
+
+  t.throws(
+    () =>
+      isolateFreeWorker({
+        ...defaultOptions,
+        dataConverter: { failureConverterPath: require.resolve('./payload-converters/failure-converter-bad-export') },
+      }),
+    {
+      message: /failureConverter export at .* must be an object with errorToFailure and failureToError methods/,
+    }
+  );
 });
 
 test('Worker with proto data converter runs an activity and reports completion', async (t) => {