[AI Improvement] [Task] Fix no-misused-promises violations (Express 4 async handlers, listeners, streams) and remove its warn override

.eslintrc.js

@@ -53,7 +53,6 @@ module.exports = {
 
         "@typescript-eslint/explicit-function-return-type": ["warn", { allowExpressions: true }], // TODO(bkendall): SET to error.
         "@typescript-eslint/no-floating-promises": "warn", // TODO(bkendall): remove, allow to error.
-        "@typescript-eslint/no-misused-promises": "warn", // TODO(bkendall): remove, allow to error.
         "@typescript-eslint/no-unsafe-argument": "warn", // TODO(bkendall): remove, allow to error.
         "@typescript-eslint/no-unsafe-assignment": "warn", // TODO(bkendall): remove, allow to error.
         "@typescript-eslint/no-unsafe-call": "warn", // TODO(bkendall): remove, allow to error.

CHANGELOG.md

@@ -4,3 +4,5 @@
 - Fixed the emulators startup message showing 127.0.0.1 for the Emulator UI and Hub when they listen on 0.0.0.0; it now also prints the address they listen on. (#6222)
 - Fixed a race condition across a second boundary during `accounts:signUp` in the Auth emulator where `validSince` could exceed `auth_time`. (#11413)
 - Added local machine config persistence to `appcheck:debugtokens:create`, `--local` support to `appcheck:debugtokens:list`, and `appcheck:debugtokens:prune` to clean up stale local debug tokens.
+- Fixed `database:import` failing with an unhandled promise rejection when a batch write fails.
+- Fixed emulator crashes when a Firestore rules reload or emulator HTTP handler throws.

src/auth.ts

@@ -69,8 +69,8 @@
     return getGlobalDefaultAccount();
   }
 
   const activeAccounts = configstore.get("activeAccounts") || {};
   const email: string | undefined = activeAccounts[projectDir];
 
   if (!email) {
     return getGlobalDefaultAccount();
@@ -84,7 +84,7 @@
  * Get all authenticated accounts _besides_ the default account.
  */
 export function getAdditionalAccounts(): Account[] {
   return configstore.get("additionalAccounts") || [];
 }
 
 /**
@@ -106,7 +106,7 @@
 /**
  * Throw an error if the provided email is not a signed-in user.
  */
 export function assertAccount(email: string, options?: { mcp?: boolean }) {
   const allAccounts = getAllAccounts();
   const accountExists = allAccounts.some((a) => a.user.email === email);
   if (!accountExists) {
@@ -126,20 +126,20 @@
  * @param options options object.
  * @param account account to make active.
  */
 export function setActiveAccount(options: any, account: Account) {
   if (account.tokens.refresh_token) {
     setRefreshToken(account.tokens.refresh_token);
   }
 
   options.user = account.user;
   options.tokens = account.tokens;
 }
 
 /**
  * Set the global refresh token in both api and apiv2.
  * @param token refresh token string
  */
 export function setRefreshToken(token: string) {
   apiv2.setRefreshToken(token);
 }
 
@@ -605,7 +605,17 @@
   getTokens: (queryCode: string, callbackUrl: string) => Promise<ResultType>,
 ): Promise<ResultType> {
   return new Promise<ResultType>((resolve, reject) => {
-    const server = http.createServer(async (req, res) => {
+    const server = http.createServer((req, res) => {
+      handleCallback(req, res).catch((err: unknown) => {
+        reject(err);
+        server.close();
+      });
+    });
+
+    async function handleCallback(
+      req: http.IncomingMessage,
+      res: http.ServerResponse,
+    ): Promise<void> {
       let searchParams: URLSearchParams;
       try {
         searchParams = new URL(req.url || "", callbackUrl).searchParams;
@@ -634,7 +644,7 @@
       }
       server.close();
       return;
-    });
+    }
 
     server.listen(port, () => {
       logger.info();

src/database/import.spec.ts

@@ -217,4 +217,11 @@ describe("DatabaseImporter", () => {
     expect(responses).to.have.length(1);
     expect(nock.isDone()).to.be.true;
   });
+
+  it("rejects with the write error when a batch write fails", async () => {
+    nock("https://test-db.firebaseio.com").get("/foo.json?shallow=true").reply(200);
+    nock("https://test-db.firebaseio.com").patch("/foo.json").reply(400, { error: "bad" });
+    const importer = new DatabaseImporter(dbUrl, DATA_STREAM, "/", payloadSize, concurrencyLimit);
+    await expect(importer.execute()).to.be.rejectedWith(FirebaseError, /400/);
+  });
 });

src/database/import.ts

@@ -186,10 +186,14 @@ export default class DatabaseImporter {
     };
 
     const writeBatch = new stream.Transform({ objectMode: true });
-    writeBatch._transform = async function (batch: SizedData, _, done) {
-      const res = await doWriteBatch(batch);
-      this.push(res);
-      done();
+    writeBatch._transform = function (batch: SizedData, _, done) {
+      void doWriteBatch(batch).then(
+        (res) => {
+          this.push(res);
+          done();
+        },
+        (err: Error) => done(err),
+      );
     };
 
     return new Promise((resolve, reject) => {

src/deploy/functions/runtimes/discovery/index.ts

@@ -162,7 +162,7 @@ export async function detectFromOutputPath(
       stderrBuffer += chunk.toString();
     });
 
-    childProcess.on("exit", async (code: number | null) => {
+    const onExit = async (code: number | null): Promise<void> => {
       if (!resolved) {
         clearTimeout(timer);
         resolved = true;
@@ -192,6 +192,11 @@ export async function detectFromOutputPath(
           }
         }
       }
+    };
+
+    // onExit catches and routes all outcomes and errors to resolve or reject
+    childProcess.on("exit", (code: number | null) => {
+      void onExit(code);
     });
 
     childProcess.on("error", (err: Error) => {

src/deploy/functions/services/index.ts

@@ -29,6 +29,9 @@ import { ensureAuthEventarcTriggerRegion } from "./authEventarc";
 /** A standard void No Op */
 export const noop = (): Promise<void> => Promise.resolve();
 
+/** A synchronous No Op, for hooks such as validateTrigger that return void */
+export const syncNoop = (): void => undefined;
+
 /** A No Op that's useful for Services that don't have specific bindings but should still try to set default bindings */
 export const noopProjectBindings = (): Promise<Array<iam.Binding>> => Promise.resolve([]);
 
@@ -73,7 +76,7 @@ const noOpService: Service = {
   name: "noop",
   api: "",
   ensureTriggerRegion: noop,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: () => Promise.resolve(FALLBACK_DEPLOYMENT_REGION),
@@ -85,7 +88,7 @@ const pubSubService: Service = {
   api: "pubsub.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: noop,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION),
@@ -97,7 +100,7 @@ const storageService: Service = {
   api: "storage.googleapis.com",
   requiredProjectBindings: obtainStorageBindings,
   ensureTriggerRegion: ensureStorageTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: ensureStorageDefaultRegion,
@@ -109,7 +112,7 @@ const firebaseAlertsService: Service = {
   api: "firebasealerts.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: ensureFirebaseAlertsTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION),
@@ -125,7 +128,7 @@ const databaseService: Service = {
   api: "firebasedatabase.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: ensureDatabaseTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: ensureDatabaseDefaultRegion,
@@ -137,7 +140,7 @@ const remoteConfigService: Service = {
   api: "firebaseremoteconfig.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: ensureRemoteConfigTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION),
@@ -149,7 +152,7 @@ const testLabService: Service = {
   api: "testing.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: ensureTestLabTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION),
@@ -161,7 +164,7 @@ const firestoreService: Service = {
   api: "firestore.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: ensureFirestoreTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: ensureFirestoreDefaultRegion,
@@ -173,7 +176,7 @@ const dataconnectService: Service = {
   api: "firebasedataconnect.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: ensureDataConnectTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: ensureDataConnectDefaultRegion,
@@ -185,7 +188,7 @@ const authEventarcService: Service = {
   api: "eventarc.googleapis.com",
   requiredProjectBindings: noopProjectBindings,
   ensureTriggerRegion: ensureAuthEventarcTriggerRegion,
-  validateTrigger: noop,
+  validateTrigger: syncNoop,
   registerTrigger: noop,
   unregisterTrigger: noop,
   getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION),

src/emulator/commandUtils.ts

@@ -265,7 +265,7 @@ function processKillSignal(
 ): SignalsListener {
   let lastSignal = new Date().getTime();
   let signalCount = 0;
-  return async () => {
+  const handleSignal = async (): Promise<void> => {
     try {
       const now = new Date().getTime();
       const diff = now - lastSignal;
@@ -339,6 +339,11 @@ function processKillSignal(
       rej(getError(e));
     }
   };
+
+  // handleSignal catches all errors and routes them to res/rej
+  return () => {
+    void handleSignal();
+  };
 }
 
 /**

src/emulator/databaseEmulator.ts

@@ -9,7 +9,7 @@ import { EmulatorInfo, EmulatorInstance, Emulators } from "../emulator/types";
 import { Constants } from "./constants";
 import { EmulatorRegistry } from "./registry";
 import { EmulatorLogger } from "./emulatorLogger";
-import { FirebaseError } from "../error";
+import { FirebaseError, getErrMsg } from "../error";
 import { connectableHostname } from "../utils";
 
 export interface DatabaseEmulatorArgs {
@@ -50,25 +50,30 @@ export class DatabaseEmulator implements EmulatorInstance {
 
         const watcher = chokidar.watch(c.rules, { persistent: true, ignoreInitial: true });
         this.rulesWatchers.push(watcher);
-        watcher.on("change", async () => {
-          // There have been some race conditions reported (on Windows) where reading the
-          // file too quickly after the watcher fires results in an empty file being read.
-          // Adding a small delay prevents that at very little cost.
-          await new Promise((res) => setTimeout(res, 5));
+        watcher.on("change", () => {
+          const update = async (): Promise<void> => {
+            // There have been some race conditions reported (on Windows) where reading the
+            // file too quickly after the watcher fires results in an empty file being read.
+            // Adding a small delay prevents that at very little cost.
+            await new Promise((res) => setTimeout(res, 5));
+
+            this.logger.logLabeled(
+              "BULLET",
+              "database",
+              `Change detected, updating rules for ${c.instance}...`,
+            );
 
-          this.logger.logLabeled(
-            "BULLET",
-            "database",
-            `Change detected, updating rules for ${c.instance}...`,
-          );
-
-          try {
-            await this.updateRules(c.instance, c.rules);
-            this.logger.logLabeled("SUCCESS", "database", "Rules updated.");
-          } catch (e: any) {
-            this.logger.logLabeled("WARN", "database", this.prettyPrintRulesError(c.rules, e));
-            this.logger.logLabeled("WARN", "database", "Failed to update rules");
-          }
+            try {
+              await this.updateRules(c.instance, c.rules);
+              this.logger.logLabeled("SUCCESS", "database", "Rules updated.");
+            } catch (e: any) {
+              this.logger.logLabeled("WARN", "database", this.prettyPrintRulesError(c.rules, e));
+              this.logger.logLabeled("WARN", "database", "Failed to update rules");
+            }
+          };
+          update().catch((err: unknown) => {
+            this.logger.logLabeled("WARN", "database", `Failed to update rules: ${getErrMsg(err)}`);
+          });
         });
       }
     }

src/emulator/dataconnect/pgliteServer.ts

@@ -49,43 +49,49 @@ export class PostgresServer {
   public async createPGServer(host = "127.0.0.1", port: number): Promise<net.Server> {
     const getDb = this.getDb.bind(this);
 
-    const server = net.createServer(async (socket) => {
-      const connection = await fromNodeSocket(socket, {
-        serverVersion: "17.4 (PGlite 0.3.3)",
-        auth: { method: "trust" },
-
-        async *onMessage(data: Uint8Array, { isAuthenticated }: { isAuthenticated: boolean }) {
-          // Only forward messages to PGlite after authentication
-          if (!isAuthenticated) {
-            return;
-          }
-          const db = await getDb();
-          if (data[0] === FrontendMessageCode.Terminate) {
-            // When the frontend terminates a connection, throw out all prepared statements
-            // because the next client won't know about them (and may create overlapping statements)
-            await db.query("DEALLOCATE ALL");
-          }
-          const response = await db.execProtocolRaw(data);
-          for await (const message of extendedQueryPatch.getMessages(data, response)) {
-            yield message;
-          }
-
-          // Extended query patch removes the extra Ready for Query messages that
-          // pglite wrongly sends.
-        },
-      });
-
-      const extendedQueryPatch: PGliteLoggerPatch = new PGliteLoggerPatch(connection);
-      socket.on("end", () => {
-        logger.debug("Postgres client disconnected");
-      });
-      // Log and destroy individual client socket errors instead of escalating them
-      // to server-level errors.
-      socket.on("error", (err) => {
-        logger.debug(
-          "Postgres client socket error (if this happened during request cancellation, it is expected and harmless):",
-          err,
-        );
+    const server = net.createServer((socket) => {
+      const handleConnection = async (): Promise<void> => {
+        const connection = await fromNodeSocket(socket, {
+          serverVersion: "17.4 (PGlite 0.3.3)",
+          auth: { method: "trust" },
+
+          async *onMessage(data: Uint8Array, { isAuthenticated }: { isAuthenticated: boolean }) {
+            // Only forward messages to PGlite after authentication
+            if (!isAuthenticated) {
+              return;
+            }
+            const db = await getDb();
+            if (data[0] === FrontendMessageCode.Terminate) {
+              // When the frontend terminates a connection, throw out all prepared statements
+              // because the next client won't know about them (and may create overlapping statements)
+              await db.query("DEALLOCATE ALL");
+            }
+            const response = await db.execProtocolRaw(data);
+            for await (const message of extendedQueryPatch.getMessages(data, response)) {
+              yield message;
+            }
+
+            // Extended query patch removes the extra Ready for Query messages that
+            // pglite wrongly sends.
+          },
+        });
+
+        const extendedQueryPatch: PGliteLoggerPatch = new PGliteLoggerPatch(connection);
+        socket.on("end", () => {
+          logger.debug("Postgres client disconnected");
+        });
+        // Log and destroy individual client socket errors instead of escalating them
+        // to server-level errors.
+        socket.on("error", (err) => {
+          logger.debug(
+            "Postgres client socket error (if this happened during request cancellation, it is expected and harmless):",
+            err,
+          );
+          socket.destroy();
+        });
+      };
+      handleConnection().catch((err: unknown) => {
+        logger.debug("Postgres client connection setup failed:", err);
         socket.destroy();
       });
     });

src/emulator/downloadableEmulators.ts

@@ -558,11 +558,12 @@ async function _runBinary(
       void handleEmulatorProcessError(emulator.name, err, command.port);
     });
 
-    emulator.instance.once("exit", async (code, signal) => {
+    emulator.instance.once("exit", (code, signal) => {
       if (signal) {
         utils.logWarning(`${description} has exited upon receiving signal: ${signal}`);
       } else if (code && code !== 0 && code !== /* SIGINT */ 130) {
-        await _fatal(emulator.name, `${description} has exited with code: ${code}`);
+        // _fatal never rejects because its finally block calls process.exit(1)
+        void _fatal(emulator.name, `${description} has exited with code: ${code}`);
       }
     });
     resolve();