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

Fixing issue with firebase init genkit on Windows #8011

Merged
merged 7 commits into from
Dec 3, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- Added default value for `emulators.dataconnect.dataDir` to `init dataconnect`.
- Fixed an issue where `firebase` would error out instead of displaying help text.
- Fixed an issue where `firebase init genkit` would error on Windows machines.
- Fixed an issue where emulator returned error when emulating alerts functions written in python (#8019)
2 changes: 1 addition & 1 deletion src/deploy/extensions/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
import * as utils from "../../utils";
import { ErrorHandler } from "./errors";
import { DeploymentInstanceSpec, InstanceSpec } from "./planner";
import { isObject } from "../../extensions/types";
import { isObject } from "../../error";

const isRetryable = (err: any) => err.status === 429 || err.status === 409;

Check warning on line 12 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function

Check warning on line 12 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type

Check warning on line 12 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value

Check warning on line 12 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value

export type DeploymentType = "create" | "update" | "configure" | "delete";
export interface ExtensionDeploymentTask {
Expand All @@ -17,14 +17,14 @@
spec: InstanceSpec;
type: DeploymentType;
}
export function extensionsDeploymentHandler(

Check warning on line 20 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
errorHandler: ErrorHandler,
): (task: ExtensionDeploymentTask) => Promise<any | undefined> {

Check warning on line 22 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
return async (task: ExtensionDeploymentTask) => {
let result;
try {
result = await task.run();
} catch (err: any) {

Check warning on line 27 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (isRetryable(err)) {
// Rethrow quota errors or operation already in progress so that throttler retries them.
throw err;
Expand All @@ -32,14 +32,14 @@
errorHandler.record(
task.spec.instanceId,
task.type,
err.context?.body?.error?.message ?? err,

Check warning on line 35 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `string`

Check warning on line 35 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .context on an `any` value
);
}
return result;
};
}

export function createExtensionInstanceTask(

Check warning on line 42 in src/deploy/extensions/tasks.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
projectId: string,
instanceSpec: DeploymentInstanceSpec,
validateOnly: boolean = false,
Expand Down
19 changes: 18 additions & 1 deletion src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,24 @@ export function getErrMsg(err: unknown, defaultMsg?: string): string {
return JSON.stringify(err);
}

function isObject(value: unknown): value is Record<string, unknown> {
/**
* Safely gets an error stack (or error message if no stack is available)
* from an unknown object
* @param err The potential error object
* @return a string representing the error stack or the error message.
*/
export function getErrStack(err: unknown): string {
if (err instanceof Error) {
return err.stack || err.message;
}
return getErrMsg(err);
}

/**
* A typeguard for objects
* @param value The value to check
*/
export function isObject(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null;
}

Expand Down
4 changes: 2 additions & 2 deletions src/extensions/localHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import * as path from "path";
import * as yaml from "yaml";

import { fileExistsSync } from "../fsutils";
import { FirebaseError } from "../error";
import { ExtensionSpec, isExtensionSpec, isObject, LifecycleEvent, LifecycleStage } from "./types";
import { FirebaseError, isObject } from "../error";
import { ExtensionSpec, isExtensionSpec, LifecycleEvent, LifecycleStage } from "./types";
import { logger } from "../logger";
import { validateSpec } from "./extensionsHelper";

Expand Down
11 changes: 6 additions & 5 deletions src/extensions/runtimes/node.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import * as path from "path";
import { execFileSync } from "child_process";
import { markedTerminal } from "marked-terminal";
import { marked } from "marked";

import { Options } from "../../options";
import { ExtensionSpec, ParamType, isObject } from "../types";
import { isObject } from "../../error";
import { ExtensionSpec, ParamType } from "../types";
import { confirm } from "../../prompt";
import * as secretsUtils from "../secretsUtils";
import { logLabeledBullet } from "../../utils";
Expand All @@ -22,6 +22,7 @@ import {
import { ALLOWED_EVENT_ARC_REGIONS } from "../askUserForEventsConfig";
import { SpecParamType } from "../extensionsHelper";
import { FirebaseError, getErrMsg } from "../../error";
import { spawnWithOutput } from "../../init/spawn";

marked.use(markedTerminal() as any);

Expand Down Expand Up @@ -473,7 +474,7 @@ export async function writeSDK(
// NPM install dependencies (since we will be adding this link locally)
logLabeledBullet("extensions", `running 'npm --prefix ${shortDirPath} install'`);
try {
execFileSync("npm", ["--prefix", dirPath, "install"]);
await spawnWithOutput("npm", ["--prefix", dirPath, "install"]);
} catch (err: unknown) {
const errMsg = getErrMsg(err, "unknown error");
throw new FirebaseError(`Error during npm install in ${shortDirPath}: ${errMsg}`);
Expand All @@ -482,7 +483,7 @@ export async function writeSDK(
// Build it
logLabeledBullet("extensions", `running 'npm --prefix ${shortDirPath} run build'`);
try {
execFileSync("npm", ["--prefix", dirPath, "run", "build"]);
await spawnWithOutput("npm", ["--prefix", dirPath, "run", "build"]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Since we don't use the output, should we be using wrapSpawn in this file instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrapSpawn requires project directory which is not available here. We may not use the output, but it doesn't really matter. Maybe in future I can refactor wrapSpawn to have project directory be optional.

} catch (err: unknown) {
const errMsg = getErrMsg(err, "unknown error");
throw new FirebaseError(`Error during npm run build in ${shortDirPath}: ${errMsg}`);
Expand All @@ -504,7 +505,7 @@ export async function writeSDK(
`running 'npm --prefix ${shortCodebaseDir} install --save ${shortDirPath}'`,
);
try {
execFileSync("npm", ["--prefix", codebaseDir, "install", "--save", dirPath]);
await spawnWithOutput("npm", ["--prefix", codebaseDir, "install", "--save", dirPath]);
} catch (err: unknown) {
const errMsg = getErrMsg(err, "unknown error");
throw new FirebaseError(`Error during npm install in ${codebaseDir}: ${errMsg}`);
Expand Down
5 changes: 1 addition & 4 deletions src/extensions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { MemoryOptions } from "../deploy/functions/backend";
import { Runtime } from "../deploy/functions/runtimes/supported";
import * as proto from "../gcp/proto";
import { SpecParamType } from "./extensionsHelper";
import { isObject } from "../error";

export enum RegistryLaunchStage {
EXPERIMENTAL = "EXPERIMENTAL",
Expand Down Expand Up @@ -281,10 +282,6 @@ export interface ParamOption {
label?: string;
}

export function isObject(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null;
}

export const isParam = (param: unknown): param is Param => {
return (
isObject(param) && typeof param["param"] === "string" && typeof param["label"] === "string"
Expand Down
19 changes: 14 additions & 5 deletions src/init/features/genkit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,23 @@ import * as inquirer from "inquirer";
import * as path from "path";
import * as semver from "semver";
import * as clc from "colorette";
import { execFileSync } from "child_process";

import { doSetup as functionsSetup } from "../functions";
import { Config } from "../../../config";
import { confirm } from "../../../prompt";
import { wrapSpawn } from "../../spawn";
import { wrapSpawn, spawnWithOutput } from "../../spawn";
import { Options } from "../../../options";
import { getProjectId } from "../../../projectUtils";
import { ensure } from "../../../ensureApiEnabled";
import { logger } from "../../../logger";
import { FirebaseError, getErrMsg } from "../../../error";
import { FirebaseError, getErrMsg, isObject } from "../../../error";
import { Setup } from "../..";
import {
logLabeledBullet,
logLabeledError,
logLabeledSuccess,
logLabeledWarning,
} from "../../../utils";
import { isObject } from "../../../extensions/types";

interface GenkitInfo {
genkitVersion: string;
Expand All @@ -61,7 +59,18 @@ async function getGenkitVersion(): Promise<GenkitInfo> {
semver.parse(process.env.GENKIT_DEV_VERSION);
genkitVersion = process.env.GENKIT_DEV_VERSION;
} else {
genkitVersion = execFileSync("npm", ["view", "genkit", "version"]).toString();
try {
genkitVersion = await spawnWithOutput("npm", ["view", "genkit", "version"]);
} catch (err: unknown) {
throw new FirebaseError(
"Unable to determine which genkit version to install.\n" +
`npm Error: ${getErrMsg(err)}\n\n` +
"For a possible workaround run\n npm view genkit version\n" +
"and then set an environment variable:\n" +
" export GENKIT_DEV_VERSION=<output from previous command>\n" +
"and run `firebase init genkit` again",
);
}
}

if (!genkitVersion) {
Expand Down
82 changes: 68 additions & 14 deletions src/init/spawn.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,88 @@
import * as spawn from "cross-spawn";
import { logger } from "../logger";
import { getErrStack, isObject } from "../error";

export function wrapSpawn(
cmd: string,
args: string[],
projectDir: string,
environmentVariables?: any,
): Promise<void> {
/**
* wrapSpawn is cross platform spawn
* @param cmd The command to run
* @param args The args for the command
* @param projectDir The current working directory to set
*/
export function wrapSpawn(cmd: string, args: string[], projectDir: string): Promise<void> {
return new Promise<void>((resolve, reject) => {
const installer = spawn(cmd, args, {
cwd: projectDir,
stdio: "inherit",
env: { ...process.env, ...environmentVariables },
env: { ...process.env },
});

installer.on("error", (err: any) => {
logger.debug(err.stack);
installer.on("error", (err: unknown) => {
logger.debug(getErrStack(err));
});

installer.on("close", (code) => {
if (code === 0) {
return resolve();
}
return reject();
return reject(
new Error(
`Error: spawn(${cmd}, [${args.join(", ")}]) \n exited with code: ${code || "null"}`,
),
);
});
});
}

/**
* spawnWithOutput uses cross-spawn to spawn a child process and get
* the output from it.
* @param cmd The command to run
* @param args The arguments for the command
* @return The stdout string from the command.
*/
export function spawnWithOutput(cmd: string, args: string[]): Promise<string> {
return new Promise((resolve, reject) => {
const child = spawn(cmd, args);

let output = "";

child.stdout?.on("data", (data) => {
if (isObject(data) && data.toString) {
output += data.toString();
} else {
output += JSON.stringify(data);
}
});

child.stderr?.on("data", (data) => {
logger.debug(
`Error: spawn(${cmd}, ${args.join(", ")})\n Stderr:\n${JSON.stringify(data)}\n`,
);
});

child.on("error", (err: unknown) => {
logger.debug(getErrStack(err));
});

child.on("close", (code) => {
if (code === 0) {
resolve(output);
} else {
reject(
new Error(
`Error: spawn(${cmd}, [${args.join(", ")}]) \n exited with code: ${code || "null"}`,
),
);
}
});
});
}

/**
* Spawn a child process with a command string.
* spawnWithCommandString spawns a child process with a command string
* @param cmd The command to run
* @param projectDir The directory to run it in
* @param environmentVariables Environment variables to set
*/
export function spawnWithCommandString(
cmd: string,
Expand All @@ -43,15 +97,15 @@ export function spawnWithCommandString(
env: { ...process.env, ...environmentVariables },
});

installer.on("error", (err: any) => {
logger.log("DEBUG", err.stack);
installer.on("error", (err: unknown) => {
logger.log("DEBUG", getErrStack(err));
});

installer.on("close", (code) => {
if (code === 0) {
return resolve();
}
return reject();
return reject(new Error(`Error: spawn(${cmd}) \n exited with code: ${code || "null"}`));
});
});
}
Loading