From e8db8a0acad1b0a8a96a34cdcc5b5939b0e3572a Mon Sep 17 00:00:00 2001 From: Eric Jizba Date: Wed, 28 Sep 2022 14:35:04 -0700 Subject: [PATCH] warn/error based on function.json files --- src/startApp.ts | 44 ++++++++++++-- test/eventHandlers/WorkerInitHandler.test.ts | 64 +++++++++++++++++++- 2 files changed, 101 insertions(+), 7 deletions(-) diff --git a/src/startApp.ts b/src/startApp.ts index f434f2f9..88bb522f 100644 --- a/src/startApp.ts +++ b/src/startApp.ts @@ -64,11 +64,45 @@ async function loadEntryPointFile(functionAppDirectory: string, channel: WorkerC } } catch (err) { const error = ensureErrorType(err); - channel.log({ - message: `Worker was unable to load entry point "${entryPointPattern}": ${error.message}`, - level: LogLevel.Warning, - logCategory: LogCategory.System, - }); + const message = `Worker was unable to load entry point "${entryPointPattern}": ${error.message}`; + + // If this is an old existing app, we can't throw an error about the entrypoint for backwards compat reasons + // More info here: https://github.com/Azure/azure-functions-nodejs-worker/issues/630 + // However, if we determine this is the new programming model, we will do the "proper" thing and throw an error + if (await isNewProgrammingModel(functionAppDirectory)) { + channel.log({ + message: + 'No "function.json" files found, so assuming this app uses a programming model requiring a valid "main" field in "package.json".', + level: LogLevel.Warning, + logCategory: LogCategory.System, + }); + + error.isAzureFunctionsSystemError = true; + error.message = message; + throw error; + } else { + channel.log({ + message, + level: LogLevel.Warning, + logCategory: LogCategory.System, + }); + } } } } + +/** + * Best effort to determine if this is the new programming model based on the existence of "function.json" files + * Defaults to false if we can't figure it out + */ +async function isNewProgrammingModel(functionAppDirectory: string): Promise { + try { + const files = await globby('*/function.json', { + cwd: functionAppDirectory, + caseSensitiveMatch: false, + }); + return files.length === 0; + } catch { + return false; + } +} diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index 2a8b9ecd..796a12fa 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -137,6 +137,15 @@ export namespace Msg { logCategory: LogCategory.System, }, }; + + export const noFunctionJsonWarning: rpc.IStreamingMessage = { + rpcLog: { + message: + 'No "function.json" files found, so assuming this app uses a programming model requiring a valid "main" field in "package.json".', + level: LogLevel.Warning, + logCategory: LogCategory.System, + }, + }; } describe('WorkerInitHandler', () => { @@ -283,13 +292,16 @@ describe('WorkerInitHandler', () => { ); }); - it('Fails for missing entry point', async function (this: ITestCallbackContext) { + it('Warns for missing entry point in old programming model', async function (this: ITestCallbackContext) { const fileName = 'entryPointFiles/missing.js'; const expectedPackageJson = { main: fileName, }; mockFs({ [__dirname]: { + HttpTrigger1: { + 'function.json': '{}', + }, 'package.json': JSON.stringify(expectedPackageJson), }, }); @@ -299,13 +311,16 @@ describe('WorkerInitHandler', () => { await stream.assertCalledWith(Msg.receivedInitLog, Msg.warning(warningMessage), Msg.response); }); - it('Fails for invalid entry point', async function (this: ITestCallbackContext) { + it('Warns for invalid entry point in old programming model', async function (this: ITestCallbackContext) { const fileName = 'entryPointFiles/throwError.js'; const expectedPackageJson = { main: fileName, }; mockFs({ [__dirname]: { + HttpTrigger1: { + 'fuNcTion.json': '{}', // try different casing as well + }, 'package.json': JSON.stringify(expectedPackageJson), // 'require' and 'mockFs' don't play well together so we need these files in both the mock and real file systems entryPointFiles: mockFs.load(path.join(__dirname, 'entryPointFiles')), @@ -321,4 +336,49 @@ describe('WorkerInitHandler', () => { Msg.response ); }); + + it('Fails for missing entry point in new programming model', async () => { + const fileName = 'entryPointFiles/missing.js'; + const expectedPackageJson = { + main: fileName, + }; + mockFs({ + [__dirname]: { + 'package.json': JSON.stringify(expectedPackageJson), + }, + }); + + stream.addTestMessage(Msg.init(__dirname)); + const errorMessage = `Worker was unable to load entry point "${fileName}": Found zero files matching the supplied pattern`; + await stream.assertCalledWith( + Msg.receivedInitLog, + Msg.noFunctionJsonWarning, + Msg.error(errorMessage), + Msg.failedResponse(fileName, errorMessage) + ); + }); + + it('Fails for invalid entry point in new programming model', async () => { + const fileName = 'entryPointFiles/throwError.js'; + const expectedPackageJson = { + main: fileName, + }; + mockFs({ + [__dirname]: { + 'package.json': JSON.stringify(expectedPackageJson), + // 'require' and 'mockFs' don't play well together so we need these files in both the mock and real file systems + entryPointFiles: mockFs.load(path.join(__dirname, 'entryPointFiles')), + }, + }); + + stream.addTestMessage(Msg.init(__dirname)); + const errorMessage = `Worker was unable to load entry point "${fileName}": test`; + await stream.assertCalledWith( + Msg.receivedInitLog, + Msg.loadingEntryPoint(fileName), + Msg.noFunctionJsonWarning, + Msg.error(errorMessage), + Msg.failedResponse(fileName, errorMessage) + ); + }); });