Skip to content

Commit

Permalink
Fix registerFunction for placeholders when indexing is on by default (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ejizba authored Mar 22, 2023
1 parent 717b0e2 commit 76aa60a
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 18 deletions.
7 changes: 7 additions & 0 deletions src/WorkerChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ export class WorkerChannel {
this.packageJson = {};
}

reset(): void {
// Ideally this resets all app-related data
// That worked is tracked by https://github.com/Azure/azure-functions-nodejs-worker/issues/670
this.workerIndexingLocked = false;
this.isUsingWorkerIndexing = false;
}

/**
* Captured logs or relevant details can use the logs property
* @param requestId gRPC message request id
Expand Down
2 changes: 2 additions & 0 deletions src/eventHandlers/FunctionEnvironmentReloadHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export class FunctionEnvironmentReloadHandler extends EventHandler<
channel: WorkerChannel,
msg: rpc.IFunctionEnvironmentReloadRequest
): Promise<rpc.IFunctionEnvironmentReloadResponse> {
channel.reset();

const response = this.getDefaultResponse(channel, msg);

// Add environment variables from incoming
Expand Down
114 changes: 99 additions & 15 deletions test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,30 @@ export namespace Msg {
export const noPackageJsonWarning: rpc.IStreamingMessage = warning(
`Worker failed to load package.json: file does not exist`
);

export function indexingSuccess(functions: rpc.IRpcFunctionMetadata[] = []): rpc.IStreamingMessage {
const response: rpc.IStreamingMessage = {
requestId: 'id',
functionMetadataResponse: {
useDefaultMetadataIndexing: functions.length === 0,
result: {
status: rpc.StatusResult.Status.Success,
},
},
};
if (functions.length > 0) {
response.functionMetadataResponse!.functionMetadataResults = functions;
}
return response;
}

export const receivedIndexingLog: rpc.IStreamingMessage = {
rpcLog: {
message: 'Worker 00000000-0000-0000-0000-000000000000 received FunctionsMetadataRequest',
level: LogLevel.Debug,
logCategory: LogCategory.System,
},
};
}

describe('FunctionEnvironmentReloadHandler', () => {
Expand All @@ -92,6 +116,7 @@ describe('FunctionEnvironmentReloadHandler', () => {

afterEach(async () => {
mock.restore();
channel.reset();
await stream.afterEachEventHandlerTest();
});

Expand Down Expand Up @@ -305,30 +330,69 @@ describe('FunctionEnvironmentReloadHandler', () => {
expect(channel.packageJson).to.deep.equal(packageJson);
});

const tempDir = 'temp';
const entryPointFilesFullPath = path.join(__dirname, 'entryPointFiles');
async function mockEntryPointFiles(fileName: string): Promise<string> {
const fileSubpath = `entryPointFiles/${fileName}`;
mock({
[tempDir]: {},
[__dirname]: {
'package.json': JSON.stringify({ main: fileSubpath }),
// 'require' and 'mockFs' don't play well together so we need these files in both the mock and real file systems
entryPointFiles: mock.load(entryPointFilesFullPath),
},
});
return fileSubpath;
}

for (const extension of ['.js', '.mjs', '.cjs']) {
it(`Loads entry point (${extension}) in specialization scenario`, async () => {
const cwd = process.cwd();
const tempDir = 'temp';
const fileName = `entryPointFiles/doNothing${extension}`;
const expectedPackageJson = {
main: fileName,
};
mock({
[tempDir]: {},
[__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: mock.load(path.join(__dirname, 'entryPointFiles')),
const fileSubpath = await mockEntryPointFiles(`doNothing${extension}`);

stream.addTestMessage(WorkerInitMsg.init(path.join(process.cwd(), tempDir)));
await stream.assertCalledWith(
WorkerInitMsg.receivedInitLog,
WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'),
WorkerInitMsg.response
);

stream.addTestMessage({
requestId: 'id',
functionEnvironmentReloadRequest: {
functionAppDirectory: __dirname,
},
});
await stream.assertCalledWith(
Msg.reloadEnvVarsLog(0),
Msg.changingCwdLog(__dirname),
WorkerInitMsg.loadingEntryPoint(fileSubpath),
WorkerInitMsg.loadedEntryPoint(fileSubpath),
Msg.reloadSuccess
);
});
}

stream.addTestMessage(WorkerInitMsg.init(path.join(cwd, tempDir)));
for (const isIndexingOnByDefault of [true, false]) {
it(`registerFunction for placeholders (indexing on by default: ${isIndexingOnByDefault})`, async () => {
const fileName = 'registerFunction.js';
const fileSubpath = await mockEntryPointFiles(fileName);
stream.addTestMessage(WorkerInitMsg.init(path.join(process.cwd(), tempDir)));
await stream.assertCalledWith(
WorkerInitMsg.receivedInitLog,
WorkerInitMsg.warning('Worker failed to load package.json: file does not exist'),
WorkerInitMsg.response
);

if (isIndexingOnByDefault) {
stream.addTestMessage({
requestId: 'id',
functionsMetadataRequest: {
functionAppDirectory: tempDir,
},
});
await stream.assertCalledWith(Msg.receivedIndexingLog, Msg.indexingSuccess());
}

stream.addTestMessage({
requestId: 'id',
functionEnvironmentReloadRequest: {
Expand All @@ -338,10 +402,30 @@ describe('FunctionEnvironmentReloadHandler', () => {
await stream.assertCalledWith(
Msg.reloadEnvVarsLog(0),
Msg.changingCwdLog(__dirname),
WorkerInitMsg.loadingEntryPoint(fileName),
WorkerInitMsg.loadedEntryPoint(fileName),
WorkerInitMsg.loadingEntryPoint(fileSubpath),
WorkerInitMsg.loadedEntryPoint(fileSubpath),
Msg.reloadSuccess
);

stream.addTestMessage({
requestId: 'id',
functionsMetadataRequest: {
functionAppDirectory: __dirname,
},
});
await stream.assertCalledWith(
Msg.receivedIndexingLog,
Msg.indexingSuccess([
{
bindings: {},
directory: entryPointFilesFullPath,
functionId: 'testFunc',
name: 'testFunc',
rawBindings: [],
scriptFile: fileName,
},
])
);
});
}
});
10 changes: 7 additions & 3 deletions test/eventHandlers/TestEventStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import { expect } from 'chai';
import { EventEmitter } from 'events';
import * as fse from 'fs-extra';
import * as path from 'path';
import * as sinon from 'sinon';
import { AzureFunctionsRpcMessages as rpc } from '../../azure-functions-language-worker-protobuf/src/rpc';
Expand Down Expand Up @@ -86,9 +87,12 @@ export class TestEventStream extends EventEmitter implements IEventStream {
}
Object.assign(process.env, this.originalEnv);

// Reset require cache for entryPoint files that depend on timing
const filePath = path.join(__dirname, 'entryPointFiles/longLoad.js');
delete require.cache[require.resolve(filePath)];
// Reset require cache for entryPoint files, otherwise they're only ever loaded once
const entryPointFilesDir = path.join(__dirname, 'entryPointFiles');
const files = await fse.readdir(entryPointFilesDir);
for (const file of files) {
delete require.cache[require.resolve(path.join(entryPointFilesDir, file))];
}

// minor delay so that it's more likely extraneous messages are associated with this test as opposed to leaking into the next test
await new Promise((resolve) => setTimeout(resolve, 20));
Expand Down
2 changes: 2 additions & 0 deletions test/eventHandlers/entryPointFiles/registerFunction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const func = require('@azure/functions-core');
func.registerFunction({ name: 'testFunc', bindings: []}, () => {});

0 comments on commit 76aa60a

Please sign in to comment.