firebase / firebase-admin-node

Firebase Admin Node.js SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
1.63k stars 371 forks source link

Realtime Database (RTDB) transaction hangs indefinitely, and after interruption by retry, clobbers data #1831

Open keanemind opened 2 years ago

keanemind commented 2 years ago

Describe your environment

When a specific sequence of events occurs involving a Realtime Database transaction, the SDK bugs out and eventually performs a write that it should not.

I am not completely sure if there is something I am not understanding about JS itself, or how the SDK works, or if there is an actual bug, and would appreciate any comments. I thought RTDB transactions work with an optimistic concurrency control mechanism, so that if the input to my transactionFn is not what is actually stored in the db, then the return value of my transactionFn will be rejected. But this principle seems to have been violated here.

Steps to reproduce:

In this scenario, everything operates on the same key in RTDB. Assume that the value for that key in the database starts off as non-null. There are a few steps that need to happen for the value to get improperly written to.

  1. .transaction(transactionFn) is invoked and the first attempt is made. The "guess" from Firebase, which is inputted into the transaction function, is null. The transactionFn returns a non-undefined value. This transaction should not take effect, because the guess does not match the value actually in the database, and indeed it does not.

  2. Second transaction attempt (internal retry by the SDK). The "guess" from Firebase is up-to-date. The transactionFn throws an error. After the error is thrown, the call .transaction(transactionFn) will hang until the next transaction attempt. By hang, I mean that the returned promise does not resolve, or if an onComplete callback is passed, it also is not called. This may be a bug in the SDK. This transaction should not take effect, because transactionFn errored, and indeed it does not.

  3. New .transaction(transactionFn) invocation. First attempt for this new invocation. Triggered by another request to the same endpoint, or in my minimal repro, triggered by the timeout. For some reason it will interrupt the stall/hang in step 2. This third call to transactionFn receives [the return value of the first transactionFn invocation] as the guess input (which may be a bug in the SDK), and returns a non-undefined value. This transaction should not take effect, because the guess does not match the value actually in the database. However, it succeeds, and the value that it returned is now in the database.

Relevant Code:

I initially encountered this issue occurring in an Express application, to which I was sending requests from a frontend. After reproducing the bug countless times, I was able to create a minimal reproducible example which does not include Express or any HTTP requests.

import readline from "readline";
import admin from "firebase-admin";
import { initializeApp, cert } from "firebase-admin/app";
import { getDatabase } from "firebase-admin/database";
import "dotenv/config";

const FIREBASE_SERVICE_ACCOUNT_KEY = process.env.FIREBASE_SERVICE_ACCOUNT_KEY;
const FIREBASE_DATABASE_URL = process.env.FIREBASE_DATABASE_URL;

process.on("uncaughtException", function (err) {
  // Without this, the process will exit after the second transaction attempt
  // causes an error to be thrown. I am not sure why I am unable to catch
  // that error from within `main()`.
  console.error("Caught exception: ", err);
});

const firebaseApp = initializeApp({
  credential: cert(JSON.parse(FIREBASE_SERVICE_ACCOUNT_KEY)),
  databaseURL: FIREBASE_DATABASE_URL,
});
const db = getDatabase(firebaseApp);

admin.database.enableLogging(true);

function sleep(ms) {
  return new Promise((resolve) => {
    setTimeout(resolve, ms);
  });
}

function prompt(rl, query) {
  return new Promise((resolve) =>
    rl.question(query, (ans) => {
      resolve(ans);
    })
  );
}

async function main() {
  const rl = readline.createInterface({
    input: process.stdin,
    output: process.stdout,
  });

  await prompt(rl, "Reset: ");
  await db.ref(`/testingFirebase`).set({
    id: 0,
  });

  await prompt(rl, "Execute transaction: ");
  /**
   * My understanding is, if the value of id in the database is initially 0,
   * and this function is the only operation modifying data in that key
   * of the database, then id should never become 2. Because id should
   * never become 1 either.
   */
  const transactionFn = (val) => {
    console.log("Transaction input:", val);
    if (val === null) {
      return {
        id: 1,
      };
    } else if (val.id === 0) {
      const a = null;
      a.thisWillError();
    } else if (val.id === 1) {
      return {
        id: 2,
      };
    }
  };

  // This transaction invocation will run first and will get stuck after the error is thrown
  try {
    db.ref(`/testingFirebase`)
      .transaction(transactionFn)
      .then(() => console.log("First transaction done"))
      .catch((e) => {
        console.log("catch", e);
        // never happens - why?
      });
  } catch (e) {
    console.log("catch", e);
    // never happens - why?
  }

  // After 3 seconds, this second transaction invocation will run and unblock the first one
  await sleep(3000)
    .then(() => console.log("Interrupting with a second transaction"))
    .then(() => db.ref(`/testingFirebase`).transaction(transactionFn));

  await prompt(rl, "Get: ");
  const snapshot = await db.ref(`/testingFirebase`).get();
  console.dir(snapshot.val());
  // { id: 2 } should have been printed, against expectations

  rl.close();
}
main();

Upon request, I can produce an example with Express, which allows one to control the order of events by sending requests rather than relying on setTimeout or readline.

digimbyte commented 1 month ago

is this issue still persistent or patched?