Skip to content
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
94 changes: 94 additions & 0 deletions src/deploy/extensions/prepare.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { expect } from "chai";
import * as sinon from "sinon";

import { prepareDynamicExtensions } from "./prepare";
import * as planner from "./planner";
import * as projectUtils from "../../projectUtils";
import * as extensionsHelper from "../../extensions/extensionsHelper";
import * as requirePermissions from "../../requirePermissions";
import { Context, Payload } from "./args";
import * as v2FunctionHelper from "./v2FunctionHelper";
import * as tos from "../../extensions/tos";

describe("Extensions prepare", () => {
describe("prepareDynamicExtensions", () => {
let haveDynamicStub: sinon.SinonStub;
let ensureExtensionsApiEnabledStub: sinon.SinonStub;
let requirePermissionsStub: sinon.SinonStub;
let needProjectIdStub: sinon.SinonStub;
let needProjectNumberStub: sinon.SinonStub;

beforeEach(() => {
haveDynamicStub = sinon.stub(planner, "haveDynamic").resolves([]);
ensureExtensionsApiEnabledStub = sinon
.stub(extensionsHelper, "ensureExtensionsApiEnabled")
.resolves();
requirePermissionsStub = sinon.stub(requirePermissions, "requirePermissions").resolves();
needProjectIdStub = sinon.stub(projectUtils, "needProjectId").returns("test-project");
needProjectNumberStub = sinon.stub(projectUtils, "needProjectNumber").resolves("123456");
});

afterEach(() => {
haveDynamicStub.restore();
ensureExtensionsApiEnabledStub.restore();
requirePermissionsStub.restore();
needProjectIdStub.restore();
needProjectNumberStub.restore();
});

it("should swallow errors and exit cleanly if the extensions API is down", async () => {
haveDynamicStub.rejects(new Error("Extensions API is having an outage"));

const context: Context = {};
const payload: Payload = {};
const options: any = {

Check warning on line 44 in src/deploy/extensions/prepare.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
config: {
src: { functions: { source: "functions" } },
},
};
const builds = {};

// This should not throw.
await expect(prepareDynamicExtensions(context, options, payload, builds)).to.not.be.rejected;

Check warning on line 52 in src/deploy/extensions/prepare.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `DeployOptions`
});

it("should proceed normally if extensions API is healthy", async () => {
haveDynamicStub.resolves([
{
instanceId: "test-extension",
ref: { publisherId: "test", extensionId: "test", version: "0.1.0" },
params: {},
systemParams: {},
labels: { codebase: "default" },
},
]);

const context: Context = {};
const payload: Payload = {};
const options: any = {

Check warning on line 68 in src/deploy/extensions/prepare.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
config: {
get: () => [],
src: { functions: { source: "functions" } },
},
rc: { getEtags: () => [] },
dryRun: true,
};
const builds = {};

const wantDynamicStub: sinon.SinonStub = sinon.stub(planner, "wantDynamic").resolves([]);
const v2apistub: sinon.SinonStub = sinon
.stub(v2FunctionHelper, "ensureNecessaryV2ApisAndRoles")
.resolves();
const tosStub: sinon.SinonStub = sinon
.stub(tos, "getAppDeveloperTOSStatus")
.resolves({ lastAcceptedVersion: "1.0.0" } as any);

Check warning on line 84 in src/deploy/extensions/prepare.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type

Check warning on line 84 in src/deploy/extensions/prepare.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `TOS | undefined`

// Expect successful completion
await expect(prepareDynamicExtensions(context, options, payload, builds)).to.not.be.rejected;

Check warning on line 87 in src/deploy/extensions/prepare.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

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

wantDynamicStub.restore();
v2apistub.restore();
tosStub.restore();
});
});
});
23 changes: 17 additions & 6 deletions src/deploy/extensions/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { Build } from "../functions/build";
import { getEndpointFilters } from "../functions/functionsDeployHelper";
import { normalizeAndValidate } from "../../functions/projectConfig";
import { DeployOptions } from "..";
import { logLabeledError } from "../../utils";

const matchesInstanceId = (dep: planner.InstanceSpec) => (test: planner.InstanceSpec) => {
return dep.instanceId === test.instanceId;
Expand Down Expand Up @@ -173,13 +174,23 @@ export async function prepareDynamicExtensions(
const projectId = needProjectId(options);
const projectNumber = await needProjectNumber(options);

await ensureExtensionsApiEnabled(options);
await requirePermissions(options, ["firebaseextensions.instances.list"]);
let haveExtensions: planner.DeploymentInstanceSpec[] = [];
try {
await ensureExtensionsApiEnabled(options);
await requirePermissions(options, ["firebaseextensions.instances.list"]);

let haveExtensions = await planner.haveDynamic(projectId);
haveExtensions = haveExtensions.filter((e) =>
extensionMatchesAnyFilter(e.labels?.codebase, e.instanceId, filters),
);
haveExtensions = await planner.haveDynamic(projectId);
haveExtensions = haveExtensions.filter((e) =>
extensionMatchesAnyFilter(e.labels?.codebase, e.instanceId, filters),
);
} catch (err) {
logLabeledError(
"extensions",
"Failed to fetch the list of extensions. Assuming for now that there are no existing extensions. " +
"If you are trying to install an extension through Firebase Functions this may fail later.",
);
return;
}
Comment on lines 186 to 193
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This catch block is too broad. It will catch any error, including permission errors (403) or billing errors (400), and incorrectly report them as an "outage". This can be misleading for users.

To make the error handling more robust, we should only suppress server-side errors (HTTP status >= 500) that are indicative of an actual outage. Other client-side errors should be re-thrown so the user is properly notified and can address the underlying issue.

  } catch (err: unknown) {
    if (err instanceof FirebaseError && err.status >= 500) {
      logLabeledError(
        "extensions",
        "Firebase Extensions is having an outage. Skipping extensions from functions codebase.",
      );
      return;
    }
    throw err;
  }


if (Object.keys(extensions).length === 0 && haveExtensions.length === 0) {
// Nothing defined, and nothing to delete
Expand Down
Loading