fix: UserError getting lost when returned from jobWorker

Co-authored-by: Johan Nyman <[email protected]>
Co-authored-by: Julian Waller <[email protected]>

Author: 3 people

meteor/__mocks__/_extendJest.ts

@@ -42,7 +42,7 @@ expect.extend({
 			received = received.error
 		}
 
-		if (UserError.isUserError(received)) {
+		if (received instanceof UserError) {
 			const expected = UserError.create(msg, args)
 			const received2 = clone(received)
 
@@ -68,10 +68,10 @@ expect.extend({
 			received = received.error
 		}
 
-		if (UserError.isUserError(received)) {
-			const pass = !!received.rawError.toString().match(regexp)
+		if (received instanceof UserError) {
+			const pass = !!received.toString().match(regexp)
 			return {
-				message: () => `expected ${received.rawError} to match ${regexp}`,
+				message: () => `expected ${stringifyError(received)} to match ${regexp}`,
 				pass: pass,
 			}
 		} else {

meteor/server/api/client.ts

@@ -34,11 +34,10 @@ import { assertConnectionHasOneOfPermissions } from '../security/auth'
 
 function rewrapError(methodName: string, e: any): ClientAPI.ClientResponseError {
 	const userError = UserError.fromUnknown(e)
-
 	logger.info(`UserAction "${methodName}" failed: ${userError.toErrorString()}`)
 
 	// Forward the error to the caller
-	return ClientAPI.responseError(userError, userError.errorCode)
+	return ClientAPI.responseError(userError)
 }
 
 export namespace ServerClientAPI {
@@ -199,7 +198,6 @@ export namespace ServerClientAPI {
 				// Just run and return right away:
 				try {
 					const result = await fcn({})
-
 					return ClientAPI.responseSuccess(result)
 				} catch (e) {
 					return rewrapError(methodName, e)

meteor/server/api/rest/v1/index.ts

@@ -50,8 +50,8 @@ koaRouter.use(bodyParser())
 
 function extractErrorCode(e: unknown): number {
 	if (ClientAPI.isClientResponseError(e)) {
-		return e.errorCode
-	} else if (UserError.isUserError(e)) {
+		return e.error.errorCode
+	} else if (UserError.isStringifiedUserErrorObject(e) || e instanceof UserError) {
 		return e.errorCode
 	} else if ((e as Meteor.Error).error && typeof (e as Meteor.Error).error === 'number') {
 		return (e as Meteor.Error).error as number
@@ -61,15 +61,17 @@ function extractErrorCode(e: unknown): number {
 }
 
 function validateUserError(e: unknown): UserError | undefined {
-	if (UserError.isUserError(e)) {
-		return e as UserError
+	if (e instanceof UserError) {
+		return e
+	} else if (UserError.isStringifiedUserErrorObject(e)) {
+		return UserError.fromUnknown(e)
 	}
 }
 
 function extractErrorUserMessage(e: unknown): string {
 	if (ClientAPI.isClientResponseError(e)) {
 		return translateMessage(e.error.userMessage, interpollateTranslation)
-	} else if (UserError.isUserError(e)) {
+	} else if (UserError.isStringifiedUserErrorObject(e) || e instanceof UserError) {
 		return translateMessage(e.userMessage, interpollateTranslation)
 	} else if ((e as Meteor.Error).reason && typeof (e as Meteor.Error).reason === 'string') {
 		return (e as Meteor.Error).reason as string
@@ -147,27 +149,27 @@ function sofieAPIRequest<API, Params, Body, Response>(
 				ctx.request.body as unknown as Body
 			)
 			if (ClientAPI.isClientResponseError(response)) {
-				// We wrap our error in another error so the status code override is not lost
-				throw UserError.from(response.error.rawError, response.error.key, undefined, response.errorCode)
+				throw UserError.fromSerialized(response.error)
 			}
 			ctx.body = JSON.stringify({ status: response.success, result: response.result })
 			ctx.status = response.success
 		} catch (e) {
 			console.log('LOOK HERE', e)
+
 			const userError = validateUserError(e)
 			const errCode = extractErrorCode(userError)
 			let errMsg = extractErrorUserMessage(userError)
 			// Get the fallback messages of the endpoint
 			const fallbackMsgs = errMsgFallbacks.get(errCode)
 
-			if (userError?.rawError.message) {
+			if (userError?.message) {
 				// If we have a detailed arbitrary error message then return that together with the standard error message.
 				errMsg = `${translateMessage(
 					{
 						key: errMsg,
 					},
 					interpollateTranslation
-				)} - ${userError?.rawError.message}`
+				)} - ${userError?.message}`
 			} else if (fallbackMsgs) {
 				// If no detailed error message is provided then return the fallback error messages.
 				const msgConcat = {

meteor/server/worker/worker.ts

@@ -22,6 +22,7 @@ import { MongoQuery } from '@sofie-automation/corelib/dist/mongo'
 import { UserActionsLog } from '../collections'
 import { MetricsCounter } from '@sofie-automation/corelib/dist/prometheus'
 import { isInTestWrite } from '../security/securityVerify'
+import { UserError } from '@sofie-automation/corelib/dist/error'
 
 const FREEZE_LIMIT = 1000 // how long to wait for a response to a Ping
 const RESTART_TIMEOUT = 30000 // how long to wait for a restart to complete before throwing an error
@@ -107,7 +108,8 @@ async function jobFinished(
 		}
 
 		if (job.completionHandler) {
-			job.completionHandler(startedTime, finishedTime, err, result)
+			const userError = err ? UserError.tryFromJSON(err) || new Error(err) : undefined
+			job.completionHandler(startedTime, finishedTime, userError, result)
 		}
 	}
 }

packages/corelib/src/error.ts

@@ -126,29 +126,23 @@ const UserErrorMessagesTranslations: { [key in UserErrorMessage]: string } = {
 	[UserErrorMessage.RateLimitExceeded]: t(`Rate limit exceeded`),
 }
 
-export interface UserErrorObj {
+export interface SerializedUserError {
 	readonly errorCode: number
 
 	/** The raw Error that was thrown */
-	readonly rawError: Error
+	rawError: Pick<Error, 'name' | 'message' | 'stack'>
 	/** The UserErrorMessage key (for matching certain error) */
 	readonly key: UserErrorMessage
 	/** The translatable string for the key */
 	readonly userMessage: ITranslatableMessage
 }
 
-export interface StringifiedUserErrorObject extends Omit<UserErrorObj, 'rawError'> {
-	rawError: Pick<Error, 'name' | 'message' | 'stack'>
-}
-
-export type StringifiedErrorObject = Omit<Error, 'cause'>
-
-export class UserError extends Error implements UserErrorObj {
+export class UserError extends Error {
 	public readonly errorCode: number
 
 	private constructor(
 		/** The raw Error that was thrown */
-		public readonly rawError: Error,
+		rawError: SerializedUserError['rawError'],
 		/** The UserErrorMessage key (for matching certain error) */
 		public readonly key: UserErrorMessage,
 		/** The translatable string for the key */
@@ -157,21 +151,31 @@ export class UserError extends Error implements UserErrorObj {
 		errorCode: number | undefined
 	) {
 		super()
-		this.errorCode = errorCode ?? 500
-	}
 
-	public toString(): string {
-		return UserError.toJSON(this)
+		// Populate the error properties:
+		this.message = rawError.message
+		this.stack = rawError.stack
+		this.name = 'UserError'
+
+		this.errorCode = errorCode ?? 500
 	}
 
 	/** Create a UserError with a custom error for the log */
 	static from(err: Error, key: UserErrorMessage, args?: { [k: string]: any }, errCode?: number): UserError {
 		return new UserError(err, key, { key: UserErrorMessagesTranslations[key], args }, errCode)
 	}
+	/** Create a UserError duplicating the same error for the log */
+	static create(key: UserErrorMessage, args?: { [k: string]: any }, errorCode?: number): UserError {
+		return UserError.from(new Error(UserErrorMessagesTranslations[key]), key, args, errorCode)
+	}
+	static fromSerialized(o: SerializedUserError): UserError {
+		return new UserError(o.rawError, o.key, o.userMessage, o.errorCode)
+	}
 	/** Create a UserError from an unknown possibly error input */
 	static fromUnknown(err: unknown, errorCode?: number): UserError {
 		if (err instanceof UserError) return err
-		if (this.isUserError(err)) {
+
+		if (this.isStringifiedUserErrorObject(err)) {
 			return new UserError(err.rawError, err.key, err.userMessage, err.errorCode)
 		}
 		const err2 = err instanceof Error ? err : new Error(stringifyError(err))
@@ -183,69 +187,61 @@ export class UserError extends Error implements UserErrorObj {
 		)
 	}
 
-	/** Create a UserError duplicating the same error for the log */
-	static create(key: UserErrorMessage, args?: { [k: string]: any }, errorCode?: number): UserError {
-		return UserError.from(new Error(UserErrorMessagesTranslations[key]), key, args, errorCode)
-	}
-
 	static tryFromJSON(str: string): UserError | undefined {
+		let p: SerializedUserError | undefined
 		try {
-			const p = UserError.fromJSON(str)
-			if (p) {
-				const newError = p.rawError as Error
-				const newUserError = new UserError(newError, p.key, p.userMessage, p.errorCode)
-				return newUserError
-			} else {
-				return undefined
-			}
+			p = UserError.fromJSON(str)
+			if (!p) return undefined
 		} catch (_e: any) {
+			// Ignore JSON parsing error
+			return undefined
+		}
+
+		if (this.isStringifiedUserErrorObject(p)) {
+			return new UserError(p.rawError, p.key, p.userMessage, p.errorCode)
+		} else {
 			return undefined
 		}
 	}
 
-	static toJSON(e: UserErrorObj): string {
-		const o: StringifiedUserErrorObject = {
+	static serialize(e: UserError): SerializedUserError {
+		const o: SerializedUserError = {
 			rawError: {
-				name: e.rawError.name,
-				message: e.rawError.message,
-				stack: e.rawError.stack,
+				name: e.name,
+				message: e.message,
+				stack: e.stack,
 			},
 			userMessage: e.userMessage,
 			key: e.key,
 			errorCode: e.errorCode,
 		}
-		return JSON.stringify(o)
+		return o
 	}
-	static fromJSON(str: string): StringifiedUserErrorObject | undefined {
+	static toJSON(e: UserError): string {
+		return JSON.stringify(this.serialize(e))
+	}
+	static fromJSON(str: string): SerializedUserError | undefined {
 		const o = JSON.parse(str)
-		if (isStringifiedUserErrorObject(o)) return o
+		if (this.isStringifiedUserErrorObject(o)) return o
 		return undefined
 	}
-
-	static isUserError(e: unknown): e is UserErrorObj {
-		return !!e && typeof e === 'object' && 'rawError' in e && 'userMessage' in e && 'key' in e
+	static isStringifiedUserErrorObject(o: unknown): o is SerializedUserError {
+		if (!o || typeof o !== 'object') return false
+		const errorObject = o as SerializedUserError
+
+		return (
+			'rawError' in errorObject &&
+			!!errorObject.rawError &&
+			typeof errorObject.rawError === 'object' &&
+			errorObject.rawError &&
+			typeof errorObject.rawError.message === 'string' &&
+			isTranslatableMessage(errorObject.userMessage) &&
+			typeof errorObject.errorCode === 'number' &&
+			typeof errorObject.key === 'number'
+		)
 	}
 
 	toErrorString(): string {
-		return `${translateMessage(this.userMessage, interpollateTranslation)}\n${stringifyError(this.rawError)}`
+		return `${translateMessage(this.userMessage, interpollateTranslation)}\n${stringifyError(this)}`
 	}
 }
-
-function isStringifiedUserErrorObject(o: any): o is StringifiedUserErrorObject {
-	return (
-		o &&
-		typeof o === 'object' &&
-		'rawError' in o &&
-		o.rawError &&
-		isStringifiedErrorObject(o.rawError) &&
-		isTranslatableMessage(o.userMessage) &&
-		typeof o.errorCode === 'number' &&
-		typeof o.key === 'number' &&
-		o.key >= 0 &&
-		o.key < Object.keys(UserErrorMessage).length / 2
-	)
-}
-
-function isStringifiedErrorObject(o: any): o is StringifiedErrorObject {
-	return o && typeof o === 'object' && 'name' in o && 'message' in o && 'stack' in o
-}

packages/job-worker/src/manager.ts

@@ -1,13 +1,12 @@
 import { WorkerId } from '@sofie-automation/corelib/dist/dataModel/Ids'
-import { UserError } from '@sofie-automation/corelib/dist/error'
 import { JobSpec } from './main.js'
 
 export interface JobManager {
 	jobFinished: (
 		id: string,
 		startedTime: number,
 		finishedTime: number,
-		error: null | Error | UserError,
+		error: null | string, // Stringified UserError
 		result: any
 	) => Promise<void>
 	// getNextJob: (queueName: string) => Promise<JobSpec>

packages/job-worker/src/playout/take.ts

@@ -72,7 +72,6 @@ export async function handleTakeNextPart(context: JobContext, data: TakeNextPart
 					)
 				}
 			}
-
 			if (lastTakeTime && now - lastTakeTime < context.studio.settings.minimumTakeSpan) {
 				logger.debug(
 					`Time since last take is shorter than ${context.studio.settings.minimumTakeSpan} for ${

packages/job-worker/src/workers/parent-base.ts

@@ -19,7 +19,7 @@ import { Promisify, ThreadedClassManager } from 'threadedclass'
 import { StatusCode } from '@sofie-automation/blueprints-integration'
 import { CollectionName } from '@sofie-automation/corelib/dist/dataModel/Collections'
 import { WorkerThreadStatus } from '@sofie-automation/corelib/dist/dataModel/WorkerThreads'
-import { UserError } from '@sofie-automation/corelib/dist/error'
+import { UserError, UserErrorMessage } from '@sofie-automation/corelib/dist/error'
 import { sleep } from '@sofie-automation/shared-lib/dist/lib/lib'
 
 export enum ThreadStatus {
@@ -306,9 +306,7 @@ export abstract class WorkerParentBase {
 										job.id,
 										startTime,
 										endTime,
-										result.error
-											? (UserError.tryFromJSON(result.error) ?? new Error(result.error))
-											: null,
+										result.error,
 										result.result
 									)
 
@@ -324,7 +322,13 @@ export abstract class WorkerParentBase {
 									logger.error(`Job errored ${job.id} "${job.name}": ${stringifyError(e)}`)
 
 									this.#watchdogJobStarted = undefined
-									await this.#jobManager.jobFinished(job.id, startTime, Date.now(), error, null)
+									await this.#jobManager.jobFinished(
+										job.id,
+										startTime,
+										Date.now(),
+										UserError.toJSON(UserError.from(error, UserErrorMessage.InternalError)),
+										null
+									)
 								}
 
 								// Ensure all locks have been freed after the job