diff --git a/src/Worker.ts b/src/Worker.ts index d2185d42..29ef46a9 100644 --- a/src/Worker.ts +++ b/src/Worker.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import * as parseArgs from 'minimist'; -import { AzFuncSystemError, ensureErrorType } from './errors'; +import { AzFuncSystemError, ensureErrorType, trySetErrorMessage } from './errors'; import { CreateGrpcEventStream } from './GrpcClient'; import { setupCoreModule } from './setupCoreModule'; import { setupEventStream } from './setupEventStream'; @@ -40,7 +40,8 @@ export function startNodeWorker(args) { } catch (err) { const error = ensureErrorType(err); error.isAzureFunctionsSystemError = true; - error.message = 'Error creating GRPC event stream: ' + error.message; + const message = 'Error creating GRPC event stream: ' + error.message; + trySetErrorMessage(error, message); throw error; } diff --git a/src/errors.ts b/src/errors.ts index b2db2759..7f8192a4 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -9,6 +9,13 @@ export interface AzFuncError { isAzureFunctionsSystemError: boolean; } +export interface ValidatedError extends Error, Partial { + /** + * Use `trySetErrorMessage` to set the error message + */ + readonly message: string; +} + export class AzFuncSystemError extends Error { isAzureFunctionsSystemError = true; } @@ -27,22 +34,8 @@ export class ReadOnlyError extends AzFuncTypeError { } } -export function ensureErrorType(err: unknown): Error & Partial { +export function ensureErrorType(err: unknown): ValidatedError { if (err instanceof Error) { - const writable = Object.getOwnPropertyDescriptor(err, 'message')?.writable; - if (!writable) { - // The motivation for this branch can be found in the below issue: - // https://github.com/Azure/azure-functions-nodejs-library/issues/205 - let readableMessage = err.message; - Object.defineProperty(err, 'message', { - get() { - return readableMessage; - }, - set(val: string) { - readableMessage = val; - }, - }); - } return err; } else { let message: string; @@ -59,6 +52,14 @@ export function ensureErrorType(err: unknown): Error & Partial { } } +export function trySetErrorMessage(err: Error, message: string): void { + try { + err.message = message; + } catch { + // If we can't set the message, we'll keep the error as is + } +} + /** * This is mostly for callbacks where `null` or `undefined` indicates there is no error * By contrast, anything thrown/caught is assumed to be an error regardless of what it is diff --git a/src/eventHandlers/FunctionLoadHandler.ts b/src/eventHandlers/FunctionLoadHandler.ts index 345c457b..f60f1196 100644 --- a/src/eventHandlers/FunctionLoadHandler.ts +++ b/src/eventHandlers/FunctionLoadHandler.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc'; -import { ensureErrorType } from '../errors'; +import { ensureErrorType, trySetErrorMessage } from '../errors'; import { loadLegacyFunction } from '../LegacyFunctionLoader'; import { isDefined, nonNullProp } from '../utils/nonNull'; import { worker } from '../WorkerContext'; @@ -43,7 +43,8 @@ export class FunctionLoadHandler extends EventHandler<'functionLoadRequest', 'fu } catch (err) { const error = ensureErrorType(err); error.isAzureFunctionsSystemError = true; - error.message = `Worker was unable to load function ${metadata.name}: '${error.message}'`; + const message = `Worker was unable to load function ${metadata.name}: '${error.message}'`; + trySetErrorMessage(error, message); throw error; } } diff --git a/src/parsers/parsePackageJson.ts b/src/parsers/parsePackageJson.ts index af6f133e..7ad25aa0 100644 --- a/src/parsers/parsePackageJson.ts +++ b/src/parsers/parsePackageJson.ts @@ -3,7 +3,7 @@ import { pathExists, readJson } from 'fs-extra'; import * as path from 'path'; -import { AzFuncSystemError, ensureErrorType } from '../errors'; +import { AzFuncSystemError, ensureErrorType, trySetErrorMessage } from '../errors'; export interface PackageJson { type?: string; @@ -36,7 +36,8 @@ export async function parsePackageJson(dir: string): Promise { } catch (err) { const error: Error = ensureErrorType(err); if (error.name === 'SyntaxError') { - error.message = `file content is not valid JSON: ${error.message}`; + const message = `file content is not valid JSON: ${error.message}`; + trySetErrorMessage(error, message); } throw error; } diff --git a/src/startApp.ts b/src/startApp.ts index 2467a319..c4e4edf2 100644 --- a/src/startApp.ts +++ b/src/startApp.ts @@ -3,7 +3,7 @@ import { AppStartContext } from '@azure/functions-core'; import { AzureFunctionsRpcMessages as rpc } from '../azure-functions-language-worker-protobuf/src/rpc'; -import { AzFuncSystemError, ensureErrorType, ReadOnlyError } from './errors'; +import { AzFuncSystemError, ensureErrorType, ReadOnlyError, trySetErrorMessage } from './errors'; import { executeHooks } from './hooks/executeHooks'; import { loadScriptFile } from './loadScriptFile'; import { parsePackageJson } from './parsers/parsePackageJson'; @@ -104,7 +104,7 @@ async function loadEntryPointFile(functionAppDirectory: string): Promise { }`; if (shouldBlockOnEntryPointError()) { - error.message = newMessage; + trySetErrorMessage(error, newMessage); error.isAzureFunctionsSystemError = true; // We don't want to throw this error now (during workerInit or funcEnvReload) because technically the worker is fine // Instead, it will be thrown during functionMetadata or functionLoad response which better indicates that the user's app is the problem diff --git a/test/errors.test.ts b/test/errors.test.ts index 096492ea..8c97006e 100644 --- a/test/errors.test.ts +++ b/test/errors.test.ts @@ -3,7 +3,7 @@ import 'mocha'; import { expect } from 'chai'; -import { ensureErrorType } from '../src/errors'; +import { ensureErrorType, trySetErrorMessage } from '../src/errors'; describe('errors', () => { it('null', () => { @@ -42,6 +42,13 @@ describe('errors', () => { expect(ensureErrorType(actualError)).to.equal(actualError); }); + it('modify error message', () => { + const actualError = new Error('test2'); + trySetErrorMessage(actualError, 'modified message'); + + expect(actualError.message).to.equal('modified message'); + }); + it('readonly error', () => { class ReadOnlyError extends Error { get message(): string { @@ -55,10 +62,11 @@ describe('errors', () => { expect(() => (actualError.message = 'exception')).to.throw(); const wrappedError = ensureErrorType(actualError); - wrappedError.message = 'Readonly error has been modified'; + const message = 'Readonly error has not been modified'; + trySetErrorMessage(wrappedError, message); - expect(wrappedError.message).to.equal('Readonly error has been modified'); - expect(wrappedError.stack).to.contain('Readonly error has been modified'); + expect(wrappedError.message).to.equal('a readonly message'); + expect(wrappedError.stack).to.not.contain('Readonly error has been modified'); }); function validateError(actual: Error, expectedMessage: string): void {