Skip to content
Open
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
Expand Up @@ -5,3 +5,4 @@
- Added web app support for Crashlytics MCP tools and prompts.
- Added support for forwarding custom HTTP headers (`Mcp-Param-*`) to remote MCP tools when defined in tool parameter input schemas (`x-mcp-header`), per [SEP-2243](https://modelcontextprotocol.io/seps/2243-http-standardization).
- Improved function parameter prompting clarity for multi-codebase deploys (#10897)
- Adds --immediate flag to ext:uninstall (#10921)
48 changes: 46 additions & 2 deletions src/commands/ext-uninstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,69 @@ import {
logPrefix,
} from "../extensions/extensionsHelper";
import { requirePermissions } from "../requirePermissions";
import { logLabeledWarning } from "../utils";
import { logLabeledBullet, logLabeledWarning, logLabeledSuccess } from "../utils";
import * as manifest from "../extensions/manifest";
import { deleteInstance } from "../extensions/extensionsApi";
import { Options } from "../options";
import { needProjectId } from "../projectUtils";
import { confirm } from "../prompt";
Comment on lines 12 to +14

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To support throwing a FirebaseError when deletion fails, we need to import FirebaseError from ../error.

Suggested change
import { Options } from "../options";
import { needProjectId } from "../projectUtils";
import { confirm } from "../prompt";
import { Options } from "../options";
import { needProjectId } from "../projectUtils";
import { confirm } from "../prompt";
import { FirebaseError } from "../error";

import { FirebaseError } from "../error";

export const command = new Command("ext:uninstall <extensionInstanceId>")
.description("uninstall an extension that is installed in your Firebase project by instance ID")
.option("--local", "deprecated")
.option(
"--immediate",
"immediately destroy GCP resources instead of waiting on next deploy. Can be run outside a firebase project directory.",
)
.withForce()
.before(requirePermissions, ["firebaseextensions.instances.delete"])
.before(ensureExtensionsApiEnabled)
.before(checkMinRequiredVersion, "extMinVersion")
.before(diagnoseAndFixProject)
.action((instanceId: string, options: Options) => {
.action(async (instanceId: string, options: Options) => {
if (options.local) {
logLabeledWarning(
logPrefix,
"As of firebase-tools@11.0.0, the `--local` flag is no longer required, as it is the default behavior.",
);
}
if (options.immediate) {
const projectId = needProjectId(options);
let config;
try {
config = manifest.loadConfig(options);
} catch {
logLabeledBullet(
logPrefix,
"No firebase.json found. Proceeding to immediate extension instance teardown.",
);
}
if (config && manifest.instanceExists(instanceId, config)) {
manifest.removeFromManifest(instanceId, config);
}
Comment on lines +38 to +49

This comment was marked as resolved.


if (
!(await confirm({
message: `About to delete Extensions instance ${projectId}/${instanceId}, its associated resources, and service account. Continue?`,
nonInteractive: options.nonInteractive,
force: options.force,
default: true,
}))
) {
return;
}
try {
await deleteInstance(projectId, instanceId);
} catch (err: unknown) {
throw new FirebaseError(
`Error when attempting deletion: ${err instanceof Error ? err.message : String(err)}`,
{ original: err instanceof Error ? err : undefined },
);
}
Comment on lines +61 to +68

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

  1. Correctness: Logging the error and returning undefined allows the command to exit with code 0 (success) even if the deletion failed. To ensure scripts and CI environments can detect failures, we should throw a FirebaseError instead.
  2. Style Guide Adherence: Use unknown instead of any in the catch clause to maintain type safety.
      try {
        await deleteInstance(projectId, instanceId);
      } catch (err: unknown) {
        throw new FirebaseError(
          `Error when attempting deletion: ${err instanceof Error ? err.message : String(err)}`,
          { original: err instanceof Error ? err : undefined }
        );
      }
References
  1. Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. (link)
  2. Throw FirebaseError (src/error.ts) for expected, user-facing errors. (link)

This comment was marked as resolved.

logLabeledSuccess(logPrefix, `Deleted Extensions instance ${projectId}/${instanceId}.`);
return;
}
const config = manifest.loadConfig(options);
manifest.removeFromManifest(instanceId, config);
});
Loading