Skip to content

Add shell debugging and fix timeout issue #1932

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

Merged
merged 1 commit into from
Jan 22, 2020
Merged
Show file tree
Hide file tree
Changes from all 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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* Adds breakpoint debugging to `functions:shell` (#1872)
* Removes function timeouts when breakpoint debugging is enabled (#1931)
4 changes: 3 additions & 1 deletion src/commands/functions-shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ var { Command } = require("../command");
var { requirePermissions } = require("../requirePermissions");
var action = require("../functionsShellCommandAction");
var requireConfig = require("../requireConfig");
var commandUtils = require("../emulator/commandUtils");

module.exports = new Command("functions:shell")
.description("launch full Node shell with emulated functions")
.option("-p, --port <port>", "the port on which to emulate functions (default: 5000)", 5000)
.option("-p, --port <port>", "the port on which to emulate functions", 5000)
.option(commandUtils.FLAG_INSPECT_FUNCTIONS, commandUtils.DESC_INSPECT_FUNCTIONS)
.before(requireConfig)
.before(requirePermissions)
.action(action);
17 changes: 17 additions & 0 deletions src/emulator/commandUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as logger from "../logger";
import requireAuth = require("../requireAuth");
import requireConfig = require("../requireConfig");
import { Emulators } from "../emulator/types";
import { FirebaseError } from "../error";

export const FLAG_ONLY: string = "--only <emulators>";
export const DESC_ONLY: string =
Expand Down Expand Up @@ -55,3 +56,19 @@ export async function beforeEmulatorCommand(options: any): Promise<any> {
await requireConfig(options);
}
}

export function parseInspectionPort(options: any): number {
let port = options.inspectFunctions;
if (port === true) {
port = "9229";
}

const parsed = Number(port);
if (isNaN(parsed) || parsed < 1024 || parsed > 65535) {
throw new FirebaseError(
`"${port}" is not a valid port for debugging, please pass an integer between 1024 and 65535.`
);
}

return parsed;
}
16 changes: 2 additions & 14 deletions src/emulator/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { HostingEmulator } from "../emulator/hostingEmulator";
import { FirebaseError } from "../error";
import * as getProjectId from "../getProjectId";
import { PubsubEmulator } from "./pubsubEmulator";
import * as commandUtils from "./commandUtils";

export const VALID_EMULATOR_STRINGS: string[] = ALL_EMULATORS;

Expand Down Expand Up @@ -133,21 +134,8 @@ export async function startAll(options: any): Promise<void> {
);

let inspectFunctions: number | undefined;

// If the flag is provided without a value, use the Node.js default
if (options.inspectFunctions === true) {
options.inspectFunctions = "9229";
}

if (options.inspectFunctions) {
inspectFunctions = Number(options.inspectFunctions);
if (isNaN(inspectFunctions) || inspectFunctions < 1024 || inspectFunctions > 65535) {
throw new FirebaseError(
`"${
options.inspectFunctions
}" is not a valid value for the --inspect-functions flag, please pass an integer between 1024 and 65535.`
);
}
inspectFunctions = commandUtils.parseInspectionPort(options);

// TODO(samstern): Add a link to documentation
utils.logLabeledWarning(
Expand Down
7 changes: 7 additions & 0 deletions src/emulator/functionsEmulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ export class FunctionsEmulator implements EmulatorInstance {
// TODO: Would prefer not to have static state but here we are!
EmulatorLogger.verbosity = this.args.quiet ? Verbosity.QUIET : Verbosity.DEBUG;

// When debugging is enabled, the "timeout" feature needs to be disabled so that
// functions don't timeout while a breakpoint is active.
if (this.args.debugPort) {
Copy link
Contributor

Choose a reason for hiding this comment

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

lgtm, can't imagine anything going wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope to use a screenshot of this comment in a P0 bug one day.

this.args.disabledRuntimeFeatures = this.args.disabledRuntimeFeatures || {};
this.args.disabledRuntimeFeatures.timeout = true;
}

const mode = this.args.debugPort
? FunctionsExecutionMode.SEQUENTIAL
: FunctionsExecutionMode.AUTO;
Expand Down
8 changes: 8 additions & 0 deletions src/functionsShellCommandAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,23 @@ var serveFunctions = require("./serve/functions");
var LocalFunction = require("./localFunction");
var logger = require("./logger");
var shell = require("./emulator/functionsEmulatorShell");
var commandUtitls = require("./emulator/commandUtils");

module.exports = function(options) {
options.port = parseInt(options.port, 10);

let debugPort = undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: let debugPort: number | undefined;

Setting something to undefined always feels dirty to me haha

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree but this is .js ... so I can't add types.

if (options.inspectFunctions) {
debugPort = commandUtitls.parseInspectionPort(options);
}

return serveFunctions
.start(options, {
// TODO(samstern): Note that these are not acctually valid FunctionsEmulatorArgs
// and when we eventually move to all TypeScript we'll have to start adding
// projectId and functionsDir here.
quiet: true,
debugPort,
})
.then(function() {
return serveFunctions.connect();
Expand Down