Use custom symbol-based impl of instanceof (#1166)

Author: mjameswh

packages/activity/src/index.ts

@@ -73,6 +73,7 @@ import 'abort-controller/polyfill'; // eslint-disable-line import/no-unassigned-
 import { AsyncLocalStorage } from 'node:async_hooks';
 import { Logger, Duration } from '@temporalio/common';
 import { msToNumber } from '@temporalio/common/lib/time';
+import { SymbolBasedInstanceOfError } from '@temporalio/common/lib/type-helpers';
 
 export {
   ActivityFunction,
@@ -82,8 +83,6 @@ export {
   UntypedActivities,
 } from '@temporalio/common';
 
-const isCompleteAsyncError = Symbol.for('__temporal_isCompleteAsyncError');
-
 /**
  * Throw this error from an Activity in order to make the Worker forget about this Activity.
  *
@@ -101,25 +100,8 @@ const isCompleteAsyncError = Symbol.for('__temporal_isCompleteAsyncError');
  *}
  *```
  */
-export class CompleteAsyncError extends Error {
-  public readonly name: string = 'CompleteAsyncError';
-
-  constructor() {
-    super();
-  }
-
-  /**
-   * Marker to determine whether an error is an instance of CompleteAsyncError.
-   */
-  protected readonly [isCompleteAsyncError] = true;
-
-  /**
-   * Instanceof check that works when multiple versions of @temporalio/activity are installed.
-   */
-  static is(error: unknown): error is CompleteAsyncError {
-    return error instanceof CompleteAsyncError || (error as any)?.[isCompleteAsyncError] === true;
-  }
-}
+@SymbolBasedInstanceOfError('CompleteAsyncError')
+export class CompleteAsyncError extends Error {}
 
 // Make it safe to use @temporalio/activity with multiple versions installed.
 const asyncLocalStorageSymbol = Symbol.for('__temporal_activity_context_storage__');

packages/client/src/async-completion-client.ts

@@ -6,6 +6,7 @@ import {
   encodeToPayloads,
   filterNullAndUndefined,
 } from '@temporalio/common/lib/internal-non-workflow';
+import { SymbolBasedInstanceOfError } from '@temporalio/common/lib/type-helpers';
 import {
   BaseClient,
   BaseClientOptions,
@@ -21,25 +22,22 @@ import { rethrowKnownErrorTypes } from './helpers';
  * Thrown by {@link AsyncCompletionClient} when trying to complete or heartbeat an Activity that does not exist in the
  * system.
  */
-export class ActivityNotFoundError extends Error {
-  public readonly name = 'ActivityNotFoundError';
-}
+@SymbolBasedInstanceOfError('ActivityNotFoundError')
+export class ActivityNotFoundError extends Error {}
 
 /**
  * Thrown by {@link AsyncCompletionClient} when trying to complete or heartbeat
  * an Activity for any reason apart from {@link ActivityNotFoundError}.
  */
-export class ActivityCompletionError extends Error {
-  public readonly name = 'ActivityCompletionError';
-}
+@SymbolBasedInstanceOfError('ActivityCompletionError')
+export class ActivityCompletionError extends Error {}
 
 /**
  * Thrown by {@link AsyncCompletionClient.heartbeat} when the Workflow has
  * requested to cancel the reporting Activity.
  */
-export class ActivityCancelledError extends Error {
-  public readonly name = 'ActivityCancelledError';
-}
+@SymbolBasedInstanceOfError('ActivityCancelledError')
+export class ActivityCancelledError extends Error {}
 
 /**
  * Options used to configure {@link AsyncCompletionClient}

packages/client/src/errors.ts

@@ -1,11 +1,12 @@
 import { ServiceError as GrpcServiceError } from '@grpc/grpc-js';
 import { RetryState, TemporalFailure } from '@temporalio/common';
+import { isError, isRecord, SymbolBasedInstanceOfError } from '@temporalio/common/lib/type-helpers';
 
 /**
  * Generic Error class for errors coming from the service
  */
+@SymbolBasedInstanceOfError('ServiceError')
 export class ServiceError extends Error {
-  public readonly name: string = 'ServiceError';
   public readonly cause?: Error;
 
   constructor(message: string, opts?: { cause: Error }) {
@@ -23,8 +24,8 @@ export class ServiceError extends Error {
  * For example if the workflow is cancelled, `cause` will be set to
  * {@link CancelledFailure}.
  */
+@SymbolBasedInstanceOfError('WorkflowFailedError')
 export class WorkflowFailedError extends Error {
-  public readonly name: string = 'WorkflowFailedError';
   public constructor(
     message: string,
     public readonly cause: TemporalFailure | undefined,
@@ -40,15 +41,19 @@ export class WorkflowFailedError extends Error {
  *
  * Only thrown if asked not to follow the chain of execution (see {@link WorkflowOptions.followRuns}).
  */
+@SymbolBasedInstanceOfError('WorkflowExecutionContinuedAsNewError')
 export class WorkflowContinuedAsNewError extends Error {
-  public readonly name: string = 'WorkflowExecutionContinuedAsNewError';
   public constructor(message: string, public readonly newExecutionRunId: string) {
     super(message);
   }
 }
 
 export function isGrpcServiceError(err: unknown): err is GrpcServiceError {
-  return err instanceof Error && (err as any).details !== undefined && (err as any).metadata !== undefined;
+  return (
+    isError(err) &&
+    typeof (err as GrpcServiceError)?.details === 'string' &&
+    isRecord((err as GrpcServiceError).metadata)
+  );
 }
 
 /**

packages/client/src/iterators-utils.ts

@@ -1,5 +1,6 @@
 import 'abort-controller/polyfill'; // eslint-disable-line import/no-unassigned-import
 import { EventEmitter, on, once } from 'node:events';
+import { isAbortError } from '@temporalio/common/lib/type-helpers';
 
 export interface MapAsyncOptions {
   /**
@@ -108,7 +109,7 @@ export async function* mapAsyncIterable<A, B>(
       yield res;
     }
   } catch (err: unknown) {
-    if ((err as Error)?.name === 'AbortError') {
+    if (isAbortError(err)) {
       return;
     }
     throw err;

packages/client/src/schedule-client.ts

@@ -9,6 +9,7 @@ import {
 } from '@temporalio/common/lib/internal-non-workflow';
 import { temporal } from '@temporalio/proto';
 import { optionalDateToTs, optionalTsToDate, optionalTsToMs, tsToDate } from '@temporalio/common/lib/time';
+import { SymbolBasedInstanceOfError } from '@temporalio/common/lib/type-helpers';
 import { CreateScheduleInput, CreateScheduleOutput, ScheduleClientInterceptor } from './interceptors';
 import { WorkflowService } from './types';
 import { isGrpcServiceError, ServiceError } from './errors';
@@ -527,9 +528,8 @@ export class ScheduleClient extends BaseClient {
  *
  * @experimental
  */
+@SymbolBasedInstanceOfError('ScheduleAlreadyRunning')
 export class ScheduleAlreadyRunning extends Error {
-  public readonly name: string = 'ScheduleAlreadyRunning';
-
   constructor(message: string, public readonly scheduleId: string) {
     super(message);
   }
@@ -543,9 +543,8 @@ export class ScheduleAlreadyRunning extends Error {
  *
  * @experimental
  */
+@SymbolBasedInstanceOfError('ScheduleNotFoundError')
 export class ScheduleNotFoundError extends Error {
-  public readonly name: string = 'ScheduleNotFoundError';
-
   constructor(message: string, public readonly scheduleId: string) {
     super(message);
   }

packages/client/src/task-queue-client.ts

@@ -1,6 +1,6 @@
 import { status } from '@grpc/grpc-js';
 import { filterNullAndUndefined } from '@temporalio/common/lib/internal-non-workflow';
-import { assertNever, RequireAtLeastOne } from '@temporalio/common/lib/type-helpers';
+import { assertNever, SymbolBasedInstanceOfError, RequireAtLeastOne } from '@temporalio/common/lib/type-helpers';
 import { temporal } from '@temporalio/proto';
 import { BaseClient, BaseClientOptions, defaultBaseClientOptions, LoadedWithDefaults } from './base-client';
 import { WorkflowService } from './types';
@@ -288,10 +288,5 @@ function reachabilityTypeFromProto(rtype: temporal.api.enums.v1.TaskReachability
  *
  * @experimental
  */
-export class BuildIdNotFoundError extends Error {
-  public readonly name: string = 'BuildIdNotFoundError';
-
-  constructor(message: string) {
-    super(message);
-  }
-}
+@SymbolBasedInstanceOfError('BuildIdNotFoundError')
+export class BuildIdNotFoundError extends Error {}

packages/client/src/workflow-client.ts

@@ -22,6 +22,7 @@ import {
 } from '@temporalio/common';
 import { composeInterceptors } from '@temporalio/common/lib/interceptors';
 import { History } from '@temporalio/common/lib/proto-utils';
+import { SymbolBasedInstanceOfError } from '@temporalio/common/lib/type-helpers';
 import {
   decodeArrayFromPayloads,
   decodeFromPayloadsAtIndex,
@@ -992,15 +993,15 @@ export class WorkflowClient extends BaseClient {
   }
 }
 
+@SymbolBasedInstanceOfError('QueryRejectedError')
 export class QueryRejectedError extends Error {
-  public readonly name: string = 'QueryRejectedError';
   constructor(public readonly status: temporal.api.enums.v1.WorkflowExecutionStatus) {
     super('Query rejected');
   }
 }
 
+@SymbolBasedInstanceOfError('QueryNotRegisteredError')
 export class QueryNotRegisteredError extends Error {
-  public readonly name: string = 'QueryNotRegisteredError';
   constructor(message: string, public readonly code: grpcStatus) {
     super(message);
   }

packages/common/src/converter/failure-converter.ts

@@ -12,7 +12,7 @@ import {
   TimeoutFailure,
   TimeoutType,
 } from '../failure';
-import { hasOwnProperties, isRecord } from '../type-helpers';
+import { isError } from '../type-helpers';
 import { arrayFromPayloads, fromPayloadsAtIndex, PayloadConverter, toPayloads } from './payload-converter';
 
 /**
@@ -219,7 +219,7 @@ export class DefaultFailureConverter implements FailureConverter {
   }
 
   errorToFailureInner(err: unknown, payloadConverter: PayloadConverter): ProtoFailure {
-    if (TemporalFailure.is(err)) {
+    if (err instanceof TemporalFailure) {
       if (err.failure) return err.failure;
       const base = {
         message: err.message,
@@ -228,7 +228,7 @@ export class DefaultFailureConverter implements FailureConverter {
         source: FAILURE_SOURCE,
       };
 
-      if (ActivityFailure.is(err)) {
+      if (err instanceof ActivityFailure) {
         return {
           ...base,
           activityFailureInfo: {
@@ -237,7 +237,7 @@ export class DefaultFailureConverter implements FailureConverter {
           },
         };
       }
-      if (ChildWorkflowFailure.is(err)) {
+      if (err instanceof ChildWorkflowFailure) {
         return {
           ...base,
           childWorkflowExecutionFailureInfo: {
@@ -247,7 +247,7 @@ export class DefaultFailureConverter implements FailureConverter {
           },
         };
       }
-      if (ApplicationFailure.is(err)) {
+      if (err instanceof ApplicationFailure) {
         return {
           ...base,
           applicationFailureInfo: {
@@ -260,7 +260,7 @@ export class DefaultFailureConverter implements FailureConverter {
           },
         };
       }
-      if (CancelledFailure.is(err)) {
+      if (err instanceof CancelledFailure) {
         return {
           ...base,
           canceledFailureInfo: {
@@ -271,7 +271,7 @@ export class DefaultFailureConverter implements FailureConverter {
           },
         };
       }
-      if (TimeoutFailure.is(err)) {
+      if (err instanceof TimeoutFailure) {
         return {
           ...base,
           timeoutFailureInfo: {
@@ -282,13 +282,13 @@ export class DefaultFailureConverter implements FailureConverter {
           },
         };
       }
-      if (ServerFailure.is(err)) {
+      if (err instanceof ServerFailure) {
         return {
           ...base,
           serverFailureInfo: { nonRetryable: err.nonRetryable },
         };
       }
-      if (TerminatedFailure.is(err)) {
+      if (err instanceof TerminatedFailure) {
         return {
           ...base,
           terminatedFailureInfo: {},
@@ -302,12 +302,12 @@ export class DefaultFailureConverter implements FailureConverter {
       source: FAILURE_SOURCE,
     };
 
-    if (isRecord(err) && hasOwnProperties(err, ['message', 'stack'])) {
+    if (isError(err)) {
       return {
         ...base,
         message: String(err.message) ?? '',
-        stackTrace: cutoffStackTrace(String(err.stack)),
-        cause: this.optionalErrorToOptionalFailure(err.cause, payloadConverter),
+        stackTrace: cutoffStackTrace(err.stack),
+        cause: this.optionalErrorToOptionalFailure((err as any).cause, payloadConverter),
       };
     }
 

packages/common/src/converter/payload-converter.ts

@@ -260,7 +260,7 @@ export class SearchAttributePayloadConverter implements PayloadConverter {
   validNonDateTypes = ['string', 'number', 'boolean'];
 
   public toPayload(values: unknown): Payload {
-    if (!(values instanceof Array)) {
+    if (!Array.isArray(values)) {
       throw new ValueError(`SearchAttribute value must be an array`);
     }
 
@@ -309,7 +309,7 @@ export class SearchAttributePayloadConverter implements PayloadConverter {
     }
 
     const value = this.jsonConverter.fromPayload(payload);
-    let arrayWrappedValue = value instanceof Array ? value : [value];
+    let arrayWrappedValue = Array.isArray(value) ? value : [value];
 
     const searchAttributeType = decode(payload.metadata.type);
     if (searchAttributeType === 'Datetime') {