TypeError: n.OpfsSAHPoolDb is not a constructor

[germanescobar]

Hi, we are getting this error intermittently and we are having a really hard time recreating it and diagnosing what's happening. Does anyone here have an idea what's going on or have a suggestion of how we can debug it.

When the error occurs it prevents our app from initializing.

• This is being called inside a Web Worker.
• We use the Web Locks API to ensure that only an instance of this Web Worker exists.

import sqlite3InitModule from "@sqlite.org/sqlite-wasm";

// The sqlite3 wasm module needs to be initialized by the browser before
// it can be used. This must only be done once. We eagerly start this
// initialization process when this module is imported.
const modulePromise = sqlite3InitModule({
  print: console.log,
  printErr: console.error,
});
export async function createPersistedClientDatabaseAdapter(props: {
  logger: Logger;
  /** Resets the database on init, clearing all existing data. */
  clearOnInit?: boolean;
}) {
  const sqlite3 = await modulePromise;
  const poolUtil = await sqlite3.installOpfsSAHPoolVfs({
    clearOnInit: props.clearOnInit,
    directory: "/sqlite3",
  });
  props.logger.info({ poolUtil });
  const sqliteDB = new poolUtil.OpfsSAHPoolDb(`/comms.db`); // intermittent error
}

[sgbeal]

To the best of my recollection, you are the first to report this particular problem, and no immediately obvious culprit comes to mind. Is it limited to a specific browser, or are you seeing this in at least two of Chrome, Firefox, and Safari?

[jorroll]

Thanks for the feedback @sgbeal. We're seeing this error in Chromium browsers including Google Chrome and Brave as well as both Desktop and Mobile Safari. There haven't been any reports of this error in Firefox, but our app only has about 100 active users at this point and, according to our logs, no one has used Firefox within the past month.

I think we've been running into this error for 3+ months now and we haven't been able to recreate it consistently. Though at this point most of our users have experienced it at least once. When it happens, our best advice at the moment is to clear the browser cache, close all open tabs, and try reopening the application. Generally this works but not always. Notably, the error can be hard to get rid of on Mobile Safari on iOS.

Edit: see my comment below for what is, I think, a more accurate description of the error we are seeing.

Expand to see original text:

Looking at the source code, the error seems related to this code inside `installOpfsSAHPoolVfs`...

Based on the source code, this error could occur if sqlite3.oo1 were falsey. Can you (or anyone else :) provide any insight into what scenarios would cause sqlite3.oo1 to be falsey 🙏 ?

// other code...
                  const poolUtil = new OpfsSAHPoolUtil(thePool);

                  if (sqlite3.oo1) {
                    const oo1 = sqlite3.oo1;
                    const theVfs = thePool.getVfs();
                    const OpfsSAHPoolDb = function (...args) {
                      const opt = oo1.DB.dbCtorHelper.normalizeArgs(...args);
                      opt.vfs = theVfs.$zName;
                      oo1.DB.dbCtorHelper.call(this, opt);
                    };

                    OpfsSAHPoolDb.prototype = Object.create(oo1.DB.prototype);

                    poolUtil.OpfsSAHPoolDb = OpfsSAHPoolDb;
                    oo1.DB.dbCtorHelper.setVfsPostOpenSql(
                      theVfs.pointer,
                      function (oo1Db, sqlite3) {
                        sqlite3.capi.sqlite3_exec(
                          oo1Db,
                          [
                            'pragma journal_mode=DELETE;',
                            'pragma cache_size=-16384;',
                          ],
                          0,
                          0,
                          0,
                        );
                      },
                    );
                  }

                  thePool.log('VFS initialized.');
                  return poolUtil;

[jorroll]

I was able to recreate the error using the Desktop Safari browser. Anecdotally I've found that the error is easier to recreate on Safari (not easy to recreate as it still is seemingly triggered randomly).

It looks like a better title for this issue is poolUtil: InvalidStateError: The object is in an invalid state..

Is there a way to recover from this error? Here's a video showing the console output after the bug has occurred.

https://www.loom.com/share/0a5034f69a0d448ab789b0f815bb60e8?sid=b4fc3b98-3c0e-4df8-8683-6afbd20c99c3

Here's the console log from Safari Console.txt. Unfortunately the formatting isn't great.

The error details from the console

poolUtil: InvalidStateError: The object is in an invalid state.
- code: 11
- message: "The object is in an invalid state."
- name: "InvalidStateError"
- sqlite3Rc: 10
- stack: ""

The stack trace for the error appears to be

The methods referenced in the stack trace are

class OpfsSAHPool {
          #logImpl(level, ...args) {
            if (this.#verbosity > level)
              loggers[level](this.vfsName + ':', ...args);
          }

          error(...args) {
            this.#logImpl(0, ...args);
          }

          storeErr(e, code) {
            if (e) {
              e.sqlite3Rc = code || capi.SQLITE_IOERR;
              this.error(e);
            }
            this.$error = e;
            return code;
          }

          async acquireAccessHandles(clearFiles) {
            const files = [];
            for await (const [name, h] of this.#dhOpaque) {
              if ('file' === h.kind) {
                files.push([name, h]);
              }
            }
            return Promise.all(
              files.map(async ([name, h]) => {
                try {
                  const ah = await h.createSyncAccessHandle();
                  this.#mapSAHToName.set(ah, name);
                  if (clearFiles) {
                    ah.truncate(HEADER_OFFSET_DATA);
                    this.setAssociatedPath(ah, '', 0);
                  } else {
                    const path = this.getAssociatedPath(ah);
                    if (path) {
                      this.#mapFilenameToSAH.set(path, ah);
                    } else {
                      this.#availableSAH.add(ah);
                    }
                  }
                } catch (e) {
                  this.storeErr(e); // <-- THE ERROR CAME FROM HERE
                  this.releaseAccessHandles();
                  throw e;
                }
              }),
            );
          }
}

cc @germanescobar

[jorroll]

Digging farther, I couldn't find a reference to InvalidStateError in this repo but after some research I see that it's a type of DOMException that can be thrown by the browser's FileSystemFileHandle API. It can be thrown by several methods, including createSyncAccessHandle, write, read, and truncate. All of these methods are used within OpfsSAHPool#acquireAccessHandles() and could be the source of this error.

Edit
After forking this package and adding a lot of additional logging, I appear to have tracked down that the error is coming from const ah = await h.createSyncAccessHandle() inside the OpfsSAHPool#acquireAccessHandles() (video of the behavior). This seems surprising since, according to the spec, this error will be thrown by createSyncAccessHandle when:

• If isInABucketFileSystem is false, queue a storage task with global to reject result with an "InvalidStateError" DOMException and abort these steps.

Looking at the File System Standard to learn what a "bucket file system" is (spec):

A FileSystemHandle is in a bucket file system if the first item of its locator's path is the empty string.

Note: This is a bit magical, but it works since only the root directory of a bucket file system can have a path which contains an empty string. See getDirectory(). All other items of a path will be a valid file name.

This explanation didn't mean much to me, but then I found a blog post with a better explanation for what a "bucket file system" is. And the blog post happened to be written by Thomas Steiner, the primary contributor to this repo, no less! Thanks for the great article Thomas!

The origin private file system (sometimes also referred to as the bucket file system) allows developers to access files that are optimized for maximum reading and writing performance.

So apparently "bucket file system" is an alias for the OPFS. Which seems to indicate that the browser thinks this file system handle isn't part of the OPFS, which seems like that has to be a browser bug? I'll also note that both times I've been able to trigger this locally today I've been using the Desktop Safari browser (though I've also personally triggered it several times on Google Chrome and Brave in the past) and the clearFiles option for acquireAccessHandles has been true (though it doesn't look like that should matter since the error appears to be thrown before the option is used).

[sgbeal]

Can you (or anyone else :) provide any insight into what scenarios would cause sqlite3.oo1 to be falsey 🙏 ?

In the canonical builds, sqlite3.oo1 literally cannot be falsy. We ostensibly have the option of building without that API but no current builds do so. If it's falsy in your environment then something beyond my ken has gone horribly wrong. If it's falsy only sometimes then something even worse has gone even more horribly wrong. That "cannot happen" in a well-behaved environment.

It looks like a better title for this issue is poolUtil: InvalidStateError: The object is in an invalid state..

Random browser-side hiccups have unfortunately proven to be a rare, difficult-to-trigger fact of life for which we have no specific cure, nor even so much as a rule of thumb for how to deal with them. The behavior you're reporting is outside the realm of the OPFS API contract, and therefore not a behavior the VFS is/can be prepared to deal with.

Sidebar: one concrete example of such an issue is that some Chrome versions will not immediately release OPFS locks when a page is reloaded, so tapping Ctrl-R on an OPFS-using page may cause the reloaded instance to hit locking errors because the first one has not been fully disposed of when the reload happens. There's nothing we can do about such problems except to tell affected folks "reload once more and it will probably work."

So apparently "bucket file system" is an alias for the OPFS.

My (mis?)understanding is that Bucket FS is a specific model for an FS and OPFS is one such FS (perhaps the only current one?). The term Bucket FS didn't enter public usage until long after we had published this VFS as the "OPFS VFS," so we (in the sqlite project) stick with the term OPFS. (It's like when Sun Microsystems decided, after releasing Java 1.2, to start calling it Java 2, it took many years for everyone else to start calling it that.)

[jorroll]

Sidebar: one concrete example of such an issue is that some Chrome versions will not immediately release OPFS locks when a page is reloaded, so tapping Ctrl-R on an OPFS-using page may cause the reloaded instance to hit locking errors because the first one has not been fully disposed of when the reload happens. There's nothing we can do about such problems except to tell affected folks "reload once more and it will probably work."

So we have run into this specific issue before and it was actually pretty easy to work around. We're initializing the sqlite3 module inside a createPersistedClientDatabaseAdapter function. That function is called by a createClientDatabase function. We simply retry initializing the sqlite adapter after a short delay if we encounter an error. Having run this in production for a few months now it's completely fixed the problem of OPFS locks not being initially released.

const adapter = await promiseRetryOnError(
  { attempts: 2, waitMs: 5000 },
  () =>
    createPersistedClientDatabaseAdapter({
      logger,
      clearOnInit: data.clearOnInit,
    }),
);

In theory I would expect something similar to work for any browser hiccups like what we're running into here. But in this case it seems like the sqlite module enters a permanent error state that can't be recovered from. It seems like this might be a flaw in the way the sqlite3 module is constructed, and not an inevitable outcome of random browser bugs. But it's hard to say since I'm still not precisely sure of what's causing this.

Thanks so much for the feedback btw :)

---

There's nothing we can do about such problems except to tell affected folks "reload once more and it will probably work."

We've also found that, sometimes, the browser won't release a Web Lock held by a tab when the tab is closed or other times when a tab is reloaded. This happens disturbingly often in Safari. It's happened a few times in Google Chrome. In Chrome's case the browser eventually released the locks (e.g. after 30 seconds), but in Safari's case it generally won't release the locks when the problem occurs. No amount of reloading will help. The only solution for Safari is generally to close all open tabs/windows wait a few seconds, and then reopen. On desktop the solution has been to steer people away from Safari. On mobile we can direct them to a PWA or Native build which only uses a single tab and avoids the need for the web locks API.

[sgbeal]

We're initializing the sqlite3 module inside a createPersistedClientDatabaseAdapter function.

i was a unfortunately sloppy with my phrasing. My intent was that we cannot account for that type of quirk at the library level. Workarounds like that unfortunately fall into the realm of higher-level code. Like the C library, our JS API is synchronous (with the very unfortunate exception of some of the OPFS-related bits), which precludes library-level workarounds like wait-and-retry.

If you can provide a case which reproduces the problem, even if only periodically, i'll be happy to look at it but can make no promises when it comes to this type of strangeness.

After having looked at @jorroll's post and video:

According to:

https://developer.mozilla.org/en-US/docs/Web/API/FileSystemFileHandle/createSyncAccessHandle

InvalidStateError DOMException

Thrown if the FileSystemSyncAccessHandle object does not represent a file in the origin private file system.

The implication of that is that the file handle from which we are attempting to extract the sync access handle (we have to have the former to get the latter) is being pulled out from under us by some <vague-hand-waving>environment-specific misbehavior</vague-hand-waving>, which falls neatly (though unsatisfactorily) into the realm of Undefined Behavior. What on earth might be causing that, aside from, e.g., a race condition with a separate tab opened for the same origin (or, similarly, a still-being-cleaned-up closed tab), i can't currently imagine :/

[jorroll]

Ah HA! I believe I have a fix of sorts (really, a work-around)!! So as previously discovered, the problem we're seeing is that, sometimes (seemingly randomly), an InvalidStateError is being thrown by this line of the sqlite initialization process:

class OpfsSAHPool {

  async acquireAccessHandles(clearFiles) {
    const files = [];
  
    for await (const [name, h] of this.#dhOpaque) {
      if ('file' === h.kind) {
        files.push([name, h]);
      }
    }
  
    return Promise.all(
      files.map(async ([name, h]) => {
        try {
          const ah = await h.createSyncAccessHandle(); // <-- Error is being thrown here

          this.#mapSAHToName.set(ah, name);
  
          // code continues...

We already try to simply re-run the initialization logic if there's an unhandled error (as mentioned above we've found this addresses some browser bugs), but that hasn't helped address this issue. Looks like the reason why is that sqlite3.installOpfsSAHPoolVfs() memoizes the result based on the vfs name, so repeated calls to the function are not being rerun. I.e.

sqlite3.installOpfsSAHPoolVfs = async function (
  options = Object.create(null),
) {
  const vfsName = options.name || optionDefaults.name;

  if (0 && 2 === ++instanceCounter) {
    throw new Error("Just testing rejection.");
  }

  if (initPromises[vfsName]) {
    return initPromises[vfsName]; // <-- the problem is that we're not rerunning the init process here
  }

  // ...

  return (initPromises[vfsName] = apiVersionCheck().then(async function () {

  // code continues...

So I tried forking this library and removing the initPromises cache so that repeated calls to sqlite3.installOpfsSAHPoolVfs() actually rerun the logic. I also added code to catch an InvalidStateError thrown during the install process, wait a few seconds, and then retry the install process. And viola! I was able to recreate the error in Desktop Safari except this time our application automatically retried sqlite3.installOpfsSAHPoolVfs() and it worked without problems the second time!

• Video of the fix

A small, unfortunate side effect is that, because of the error during the install process, await thePool.removeVfs().catch(() => {}) is called which I believe clears the database, but in our case that's not a big deal. I'm also not sure if removing the vfs is strictly necessary when handling this particular error, so a further optimization in the future might be to add some logic that attempts to skip thePool.removeVfs() when handling certain errors, but that's less important.

The big headline for us is that we don't need to ask the user to try reloading the page 🙌.

I imagine that the initPromises[vfsName] cache is being used because, if the vfs was being actively used by a sqlite database at the time calling sqlite3.installOpfsSAHPoolVfs() again would cause some sort of issue, but we've found that the inability to rerun sqlite3.installOpfsSAHPoolVfs() in the case of random browser issues seems to be a larger problem.

Would this repo consider an update to either remove the initPromises concept entirely from sqlite3.installOpfsSAHPoolVfs() (in favor of just telling developers "don't call this twice if the vfs is being used by a database") or adding an property to the installOpfsSAHPoolVfs() options object that allows ignoring initPromises[vfsName] and rerunning the logic @sgbeal? Personally I'd vote for just removing the initPromises concept entirely in and trust developers to decide if they want to rerun the installation process.

---

Edit: nevermind, looks like this second issue is already fixed on main in this repo.

Identifying this bug was made harder because of this line in the `sqlite3.installOpfsSAHPoolVfs()` source

  .catch(async (e) => {
    await thePool.removeVfs().catch(() => { });
    return e; // <--
  });

Note that the error is being returned rather than thrown. This was surprising for us and is why the error was being surfaced as TypeError: n.OpfsSAHPoolDb is not a constructor.

---

i was a unfortunately sloppy with my phrasing. My intent was that we cannot account for that type of quirk at the library level. Workarounds like that unfortunately fall into the realm of higher-level code. Like the C library, our JS API is synchronous (with the very unfortunate exception of some of the OPFS-related bits), which precludes library-level workarounds like wait-and-retry.

Gotcha. Though since the error is being thrown during the initialization process, which happens in javascript and is already async, in theory it should be possible to catch some of these errors and retry after a delay.

[sgbeal]

Looks like the reason why is that sqlite3.installOpfsSAHPoolVfs() memoizes the result based on the vfs name, so repeated calls to the function are not being rerun. I.e.

That's very intentional to avoid undefined behavior if the init is run twice. Removing that memo will eventually cause someone grief. The interface specifically supports clients calling it subsequent times in order to fetch the associated PoolUtil object, in case, e.g., the shape of their API does not enable them to get a reference to it via a post-init then() handler. From the API docs:

installOpfsSAHPoolVfs() returns a Promise which resolves, on success, to a utility object which can be used to perform basic administration of the file pool (colloquially known as PoolUtil). Calling installOpfsSAHPoolVfs() more than once will resolve to the same value on second and subsequent invocations so long as the same name option is used in each invocation.

Even if removing the memo would be a good idea (though i opine that it's not!), this project's strict backwards-compatibility policy would require us to keep it.

What we can do, though, is...

...adding an property to the installOpfsSAHPoolVfs() options object that allows ignoring initPromises[vfsName]

with a big fat doc caveat stating users should never actually do that.

Personally I'd vote for just removing the initPromises concept entirely in and trust developers to decide if they want to rerun the installation process.

It's not so much about trusting the developer to not call it twice, but to enable them to call it any number of times with well-defined results. The motivating concept was to enable users at all levels of the API, from those working directly with the library to those using a higher-level application, to be able to get ahold of the relevant PoolUtil object or, if init failed, to be able to tell that init failed (by getting the cached failed promise back). The inability to double-init the VFS for a given slice of storage (i.e. a given VFS name) is very intentional: if the environment fails to initialize the VFS, the library has to assume that the environment is borked. There is no sane environment which will sometimes initialize and sometimes mysteriously not.

I'm also not sure if removing the vfs is strictly necessary when handling this particular error, so a further optimization in the future might be to add some logic that attempts to skip thePool.removeVfs() when handling certain errors, but that's less important.

The problem is, as you allude to, that we don't know which of those "certain errors" we should be doing that for and which ones not, and we don't know what future mysterious errors may crop up. In the case of init failure, we need to (A) assume that the environment is unfit for our purpose and (B) leave the VFS's storage in a known state, and if we don't clean up then we're leaving it in an undefined state.

In any case, here are the preliminary docs for the new flag, with the bold part denoting a difference from your initial proposal:

• forceReinitIfFailed: (default=false) Is a fallback option to assist in working around certain flaky environments which may mysteriously fail to provide OPFS access on an initial attempt but recover from that failure on a second attempt. This option should never be used but is provided for those who choose to throw caution to the wind and trust such environments. If this option is truthy and a previous attempt to initialize this VFS with the same name failed, that failure will be reset and the VFS init will be attempted again.

The intent of the bolded part is to reduce the "blast radius" of this nuclear option. i'm open to debating that part of its description if you like (and renaming the flag if that part is changed or removed).

It's intentionally a bit vague on when it should be used (A) to dissuade folks from using it and (B) to future-proof it against similar future cases which may come up.

Does that sound like a viable approach to you?

[sgbeal]

He's a preliminary patch for the part of installOpfsSAHPoolVfs() which returns the cached result. If you can try it out and let us know if that will do the job for you, it would be appreciated:

    if(initPromises[vfsName]){
      const p = initPromises[vfsName];
      if( (p instanceof OpfsSAHPool) || !options.forceReinitIfFailed ){
        return p;
      }
      delete initPromises[vfsName];
      /* Fall through and try again. */
    }

It occurs to me now that this may introduce a new race condition when the new flag is used and the installation is run twice concurrently, but i'm sorely tempted to hand-wave that away as "if this flag is used, all bets are off."

[sgbeal]

The prerelease snapshot has been updated with this change, which is currently in a branch pending buy-in and/or change suggestions from those involved in this discussion. The new flag's name is currently forceReinitIfFailed but suggestions for a more appropriate name are welcomed.

[jorroll]

Does that sound like a viable approach to you?

In general, yes :). Though I think there's an issue with the current implementation.

It occurs to me now that this may introduce a new race condition when the new flag is used and the installation is run twice concurrently, but i'm sorely tempted to hand-wave that away as "if this flag is used, all bets are off."

Just looking at the implementation snippet you shared, I don't think this will work because const p = initPromises[vfsName] will be a promise or undefined @sgbeal. I.e. I believe (p instanceof OpfsSAHPool) will always be false.

Maybe something like the following:

    if (initPromises[vfsName]) {
      try {
        // `await` is necessary for errors to be caught here
        return await initPromises[vfsName];
      } catch (e) {
        if (options.forceReinitIfFailed) {
           /* Fall through and try again in this case. */
           delete initPromises[vfsName];
        } else {
          throw e;
        }
      }
    }

I believe the above fix also addresses the race condition.

It's not so much about trusting the developer to not call it twice, but to enable them to call it any number of times with well-defined results.
...
the case of init failure, we need to (A) assume that the environment is unfit for our purpose and (B) leave the VFS's storage in a known state, and if we don't clean up then we're leaving it in an undefined state.
...
There is no sane environment which will sometimes initialize and sometimes mysteriously not.

This makes sense 👍

In any case, here are the preliminary docs for the new flag, with the bold part denoting a difference from your initial proposal:
forceReinitIfFailed: ...

Seems like a sensible approach 👍.

[sgbeal]

Just looking at the implementation snippet you shared, I don't think this will work because const p = initPromises[vfsName] will be a promise or undefined...

We have tests in that checkin which demonstrate (unless i've completely mis-formulated them) it working as hoped. See the bottom half of https://sqlite.org/src/info/5286e0f654d91a4e.

Edit: correction - one of the tests was flawed and correcting it reveals the bug you mentioned.

Even so, i agree that your approach looks like it would alleviate the potential race condition which i suspect (but am not certain) exists in the current impl, so it will be reformulated along those lines.

i will post a follow-up and a new snapshot build when this is done, but it will probably be tomorrow (Friday).

Thank you for taking a critical look at it!

[sgbeal]

Okay, that went quickly. There's a new prerelease snapshot with the above-discussed changes, and the tests look to be more correct this time around.

Please try that out and let us know if that will suffice for you (and whether i've made any boneheaded mistakes).

Again, thank you for taking the time to test this out and provide feedback!

Edit: for completeness's sake, the corresponding checkin is: https://sqlite.org/src/info/c4f468309158f9b9