Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request upgrades the firebase-admin dependency to ^14.5.0 and refactors various integration and conformance test files to adopt the modular Firebase Admin SDK APIs. The review feedback identifies a common issue across several conformance tests where calling deleteApp(getApp()) in the after hook can throw a FirebaseAppError if the before hook fails prior to initialization, which masks the actual setup error. To resolve this, the reviewer suggests importing getApps and safely cleaning up any initialized apps using getApps().map(app => deleteApp(app)).
| import { Bucket } from "@google-cloud/storage"; | ||
| import { expect } from "chai"; | ||
| import * as admin from "firebase-admin"; | ||
| import { applicationDefault, cert, deleteApp, getApp, initializeApp } from "firebase-admin/app"; |
There was a problem hiding this comment.
| }); | ||
|
|
||
| after(async function (this) { | ||
| this.timeout(EMULATORS_SHUTDOWN_DELAY_MS); |
There was a problem hiding this comment.
Using deleteApp(getApp()) in the after hook will throw a FirebaseAppError if the before hook failed before initializeApp was called. This masks the actual setup error with a confusing "The default Firebase app does not exist" error. Use getApps() to safely delete any initialized apps instead.
await Promise.all(getApps().map((app) => deleteApp(app)));| import { Bucket, CopyOptions } from "@google-cloud/storage"; | ||
| import { expect } from "chai"; | ||
| import * as admin from "firebase-admin"; | ||
| import { applicationDefault, cert, deleteApp, getApp, initializeApp } from "firebase-admin/app"; |
| @@ -65,7 +66,7 @@ describe("GCS Javascript SDK conformance tests", () => { | |||
|
|
|||
| after(async function (this) { | |||
| this.timeout(EMULATORS_SHUTDOWN_DELAY_MS); | |||
There was a problem hiding this comment.
Using deleteApp(getApp()) in the after hook will throw a FirebaseAppError if the before hook failed before initializeApp was called. This masks the actual setup error with a confusing "The default Firebase app does not exist" error. Use getApps() to safely delete any initialized apps instead.
await Promise.all(getApps().map((app) => deleteApp(app)));| import { Bucket } from "@google-cloud/storage"; | ||
| import { expect } from "chai"; | ||
| import * as admin from "firebase-admin"; | ||
| import { applicationDefault, cert, deleteApp, getApp, initializeApp } from "firebase-admin/app"; |
| @@ -75,7 +76,7 @@ describe("GCS endpoint conformance tests", () => { | |||
|
|
|||
| after(async function (this) { | |||
| this.timeout(EMULATORS_SHUTDOWN_DELAY_MS); | |||
There was a problem hiding this comment.
Using deleteApp(getApp()) in the after hook will throw a FirebaseAppError if the before hook failed before initializeApp was called. This masks the actual setup error with a confusing "The default Firebase app does not exist" error. Use getApps() to safely delete any initialized apps instead.
await Promise.all(getApps().map((app) => deleteApp(app)));| import * as puppeteer from "puppeteer"; | ||
| import { expect } from "chai"; | ||
| import * as admin from "firebase-admin"; | ||
| import { applicationDefault, cert, deleteApp, getApp, initializeApp } from "firebase-admin/app"; |
| after(async function (this) { | ||
| this.timeout(EMULATORS_SHUTDOWN_DELAY_MS); | ||
| admin.app().delete(); | ||
| await deleteApp(getApp()); |
There was a problem hiding this comment.
Using deleteApp(getApp()) in the after hook will throw a FirebaseAppError if the before hook failed before initializeApp was called. This masks the actual setup error with a confusing "The default Firebase app does not exist" error. Use getApps() to safely delete any initialized apps instead.
await Promise.all(getApps().map((app) => deleteApp(app)));
Resolves Buganizer b/567662510 (parent goal: b/567660818)
Proposed Improvement
firebase-admindevDependency from^11.5.0to^14.5.0.npm auditfrom@google-cloud/firestore6.x →google-gax4.x →protobufjs-cli(4 advisories) and older nestedprotobufjs.overrides.firebase-admin(pinninguuid: ^11.1.1), asfirebase-admin@14no longer bundlesuuidor@google-cloud/firestoredirectly."firebase-functions": { "firebase-admin": "-admin" }to satisfyfirebase-functions@4.3.1peerDependency requirement without breaking resolution.firebase-admin/app,firebase-admin/storage,firebase-admin/database,firebase-admin/auth,firebase-admin/firestore).npm-shrinkwrap.jsonunder Node 24 usingnpx -y npm@11.9 install --package-lock-only --ignore-scriptswith zero drift.Verification
npm run build: Passed cleanly (built MCP apps and compiled TypeScript).npm run test:compile: Passed cleanly (0 TypeScript errors across the repository).npm run lint:quiet: Passed cleanly across the entire repository (0 errors).npm run generate:json-schema && git diff --exit-code -- schema/: Verified schema consistency.adminSdkConfig,functionsEmulatorShared,functionsEmulatorUtils, andfunctionsRuntimeWorkerpassed cleanly (51 passing).npm audit: Completely clearedfirebase-admin,@google-cloud/firestore,google-gax, andprotobufjs-clivulnerabilities.