kevlened / fireway

A schema migration tool for firestore
MIT License
277 stars 41 forks source link

"fireway detected open async calls" when creating documents #74

Open bradleesand opened 1 year ago

bradleesand commented 1 year ago

For some reason, I keep getting warnings that "fireway detected open async" calls whenever I create a document in a migration. All of these example migrations that do manage their async calls properly, are getting that warning.

module.exports.migrate = async ({ firestore: db }) => {
    await db.collection('test').add({ foo: 'bar' });
};
module.exports.migrate = async ({ firestore: db }) => {
    await db.collection('test').doc().set({ foo: 'bar' });
};
module.exports.migrate = ({ firestore: db }) => {
   db.collection('test').doc();
};
j1mmie commented 1 year ago

Same here. Happens with batches too.

Node version: v16.20.0 Fireway SHA: master (ed478ff)

tciuro commented 1 year ago

+1

Interesting that hard-coding the docID works:

module.exports.migrate = async ({ firestore, FieldValue, FieldPath }) => {
    await firestore.collection('test').doc('foo').set({key: 'value'})
}

But as reported by @bradleesand, any other variation fails.

j1mmie commented 9 months ago

Made some headway on this issue:

  1. Found out that Firestore's internal method, autoid, is the culprit. It calls crypto.randomBytes, which is reporting an open async call but not reporting that it is closed.
  2. Found a similar issue in Jest: https://github.com/uuidjs/uuid/issues/566
  3. The issue was resolved in Jest here: https://github.com/jestjs/jest/pull/11278/files. It simply checks the type of async hook that is triggered, and ignores type RANDOMBYTESREQUEST

I tried applying a similar fix to Fireway, but for some reason the async hook for the get promise (or whichever method is used) gets lumped into the same async hook as the randomBytes promise. I can't find a way to both safely detect open-async calls and ignore RANDOMBYTESREQUEST calls.

I'll continue looking for workarounds / fixes.

j1mmie commented 9 months ago

Created a workaround. It ain't pretty, but it's available here:

https://github.com/j1mmie/fireway

It works by shimming the crypto module's randomBytes method. The shimmed version does not dispatch RANDOMBYTESREQUEST hooks, but still strives to be cryptographically secure.

Firestore's client library uses the autoid function (which calls randomBytes) to attach a tag to every network request. That's why it gets called even in get and so forth. It also uses autoid when creating a random document id. So if you're at all concerned about these tags/ids being cryptographically secure, then I would not use my hack. Initial entropy is obtained from the crypto module itself. And the PRNG comes from best practices by hash/prng guru @bryc. But I am in no way qualified to offer any sort of guarantee.

My fork also fixes an issue with forceAwait not working, and adds a 30 second timeout to it. It might possibly resolve this: https://github.com/kevlened/fireway/issues/51