Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add ability to stop containerless function #4025

Merged
merged 12 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/salesforcedx-vscode-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"Other"
],
"dependencies": {
"@heroku/functions-core": "^0.2.7",
"@heroku/functions-core": "^0.3.0",
"@salesforce/core": "^2.35.0",
"@salesforce/salesforcedx-sobjects-faux-generator": "54.11.0",
"@salesforce/salesforcedx-utils-vscode": "54.11.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const CONTAINER_START_TEXT_KEY =
export const FUNCTION_CONTAINER_LOG_NAME = 'force_function_containerless_start';

/**
* Executes sfdx run:function:start:local --verbose
* Starts a local run of the function which can then be invoked with payloads.
* @param sourceUri
*/
export const forceFunctionContainerlessStartCommand = async (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ export class ForceFunctionContainerStartExecutor extends ForceFunctionStartExecu
});
}

public startFunction(functionName: string): void {
public async startFunction(functionName: string): Promise<void> {
channelService.appendLine(`Starting ${functionName} in container`);
if (!this.functionsBinary) {
throw new Error('Unable to start function with no binary.');
}

this.functionsBinary.run(functionName, {}).catch(err => {
await this.functionsBinary.run(functionName, {}).catch(err => {
console.log(err);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { LocalRun } from '@heroku/functions-core';
import { LocalRun, LocalRunProcess } from '@heroku/functions-core';
import { Disposable } from 'vscode';
import { channelService } from '../../../channels';
import { nls } from '../../../messages';
Expand All @@ -19,22 +19,27 @@ import {
import { ForceFunctionStartExecutor } from './ForceFunctionStartExecutor';

export class ForceFunctionContainerlessStartExecutor extends ForceFunctionStartExecutor {
private process: LocalRunProcess | undefined | void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: void should encompass undefined here so I think it could just be LocalRunProcess | void

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing void results in error at this.process = await localRun.exec()

(property) ForceFunctionContainerlessStartExecutor.process: LocalRunProcess | undefined
Type 'void | LocalRunProcess' is not assignable to type 'LocalRunProcess | undefined'.
  Type 'void' is not assignable to type 'LocalRunProcess | undefined'.ts(2322)

Removing undefined results in error at private process: LocalRunProcess | void;

(property) ForceFunctionContainerlessStartExecutor.process: void | LocalRunProcess
Property 'process' has no initializer and is not definitely assigned in the constructor.ts(2564)

And looks like void can't be assigned to e.g. this.process = undefined; -> this.process = void; results in error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

word. Thanks for trying.


public async setupFunctionListeners(): Promise<void> {
console.log('No listeners for containerless function.');
}

public async cancelFunction(
registeredStartedFunctionDisposable: Disposable
): Promise<void> {
// TODO: how to stop the localRun
if (this.process && !this.process.cancelled) {
this.process.cancel();
this.process = undefined;
}
registeredStartedFunctionDisposable.dispose();
}

public async buildFunction(): Promise<void> {
console.log('No build for containerless function');
}

public startFunction(functionName: string, functionDirPath: string): void {
public async startFunction(functionName: string, functionDirPath: string): Promise<void> {
const functionLanguage = FunctionService.instance.getFunctionType();
channelService.appendLine(
`Starting ${functionName} of type ${functionLanguage}`
Expand All @@ -49,13 +54,7 @@ export class ForceFunctionContainerlessStartExecutor extends ForceFunctionStartE
const debugType = functionLanguage === functionType.JAVA ? 'java' : 'node';
FunctionService.instance.updateFunction(functionDirPath, debugType, true);

localRun
.exec()
.then(msg => {
console.log(
`localRun resolved in ForceFunctionContainerlessStartExecutor with message: ${msg}`
);
})
this.process = await localRun.exec()
.catch((err: Error) => {
const errorNotificationMessage = nls.localize(
this.UNEXPECTED_ERROR_KEY
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export abstract class ForceFunctionStartExecutor extends LibraryCommandletExecut
this.buildFunction(functionName, functionDirPath);

channelService.appendLine(`Starting ${functionName}`);
this.startFunction(functionName, functionDirPath);
await this.startFunction(functionName, functionDirPath);
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { LocalRunProcess } from '@heroku/functions-core';
import { vscodeStub } from '@salesforce/salesforcedx-utils-vscode/out/test/unit/commands/mocks';
import { expect } from 'chai';
import * as proxyquire from 'proxyquire';
Expand Down Expand Up @@ -216,19 +217,14 @@ describe('ForceFunctionContainerlessStartExecutor unit tests', () => {
});

it('Should be able to call methods that are no-ops for containerless mode.', async () => {
const disposable = { dispose: stub() };
const executor = new ForceFunctionContainerlessStartExecutor(
START_KEY,
LOG_NAME
);
const listenerResult = await executor.setupFunctionListeners(
'funDirPath',
disposable
'funDirPath'
);
expect(listenerResult).to.equal(undefined);
const cancelResult = await executor.cancelFunction(disposable);
expect(cancelResult).to.equal(undefined);
assert.calledOnce(disposable.dispose);
const buildResult = await executor.buildFunction('nameMe', 'funDirPath');
expect(buildResult).to.equal(undefined);
});
Expand Down Expand Up @@ -260,7 +256,32 @@ describe('ForceFunctionContainerlessStartExecutor unit tests', () => {
assert.calledOnce(fakeLocalRunInst.exec);
});

it('Should call telementry when localRun fails.', async () => {
it('Should be able to cancel running function.', async () => {
const fakeProcess = { cancel: stub().resolves() };
const fakeLocalRunInst = {
exec: stub().resolves(fakeProcess)
};
const disposable = {
dispose: stub()
};
const executor = new ForceFunctionContainerlessStartExecutor(
START_KEY,
LOG_NAME
);
localRunConstructorStub.returns(fakeLocalRunInst);

// Sets the local process
await executor.startFunction('foo', 'bar');

// have to wait for the unawaited promise in exec().then() to resolve
await Promise.resolve();

await executor.cancelFunction(disposable);
assert.calledOnce(fakeProcess.cancel);
assert.calledOnce(disposable.dispose);
});

it('Should call telemetry when localRun fails.', async () => {
const fakeType = 'typescript';
getFunctionTypeStub.returns(fakeType);
const errMessage = 'oh noes. FAIL';
Expand Down