sqlite / sqlite-wasm

SQLite Wasm conveniently wrapped as an ES Module.
520 stars 46 forks source link

False positive warning: Ignoring inability to install OPFS sqlite3_vfs #62

Closed steida closed 7 months ago

steida commented 7 months ago

I switched to the "opfs-sahpool", removed COOP/COEP HTTP headers, everything works except the library is still telling me: SqliteLive.js:105 Ignoring inability to install OPFS sqlite3_vfs:

I think it's a bug.

sgbeal commented 7 months ago

The warning is for the other OPFS VFS, not sahpool.

steida commented 7 months ago

Yes, but why does it warn me when I use the sahpool?

sgbeal commented 7 months ago

It happens during library bootstrapping, long before sahpool registration. The two are independent and don't know about each other.

steida commented 7 months ago

So do you think I should leave this in my code using sahpool only?

// https://github.com/sqlite/sqlite-wasm/issues/62
// eslint-disable-next-line no-console
const originalConsoleWarn: (...data: any[]) => void = console.warn;
// eslint-disable-next-line no-console
console.warn = (...data: any[]): void => {
  if (
    typeof data[0] === "string" &&
    data[0].startsWith("Ignoring inability to install OPFS")
  )
    return;
  // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
  originalConsoleWarn(...data);
};
sgbeal commented 7 months ago

i don't have an opinion on that. The warning is there to help me in supporting the library by giving me more info than the "it doesn't work" which users so commonly post.

There is a config option to replace the library's logging routines, but i don't recall off hand how do it and won't be back on a computer until the morning. i'll post back with instructions then.

sgbeal commented 7 months ago

It turns out that the option to replace the library's loggers was added specifically to squelch that very warning when you reported it about a year ago:

https://sqlite.org/forum/forumpost/fd9c895b1236cf65

Unfortunately, i have completely neglected to document that option anywhere except in the in-js-file docs (which we don't generally expect folks to read), but will get it added to the /wasm site docs later today.

sgbeal commented 7 months ago

FYI, the doc site now includes instructions for replacing the loggers in the cookbook (for lack of a more obvious place to put it (suggestions are welcomed)):

https://sqlite.org/wasm/doc/trunk/cookbook.md

steida commented 7 months ago

I was looking for log docs and found nothing. Thank you for the update.

I suggest not warning about the unused feature because it doesn't make sense, and false positives are confusing. Another suggestion is that the global object shouldn't be misused for configuration.

sgbeal commented 7 months ago

I suggest not warning about the unused feature because it doesn't make sense,

It makes sense for those supporting the library when users come saying "it doesn't work" and we need more info. We can't know, in advance, whether it will be used or not, but OPFS is the main reason for people wanting to use sqlite in the browser, so warning when it cannot work seems appropriate.

Another suggestion is that the global object shouldn't be misused for configuration.

Agreed 100%, but we have no other way to feed the config to the library because of the ordering of how Emscripten will initialize the module. The ability to set a global variable to configure the library is an ugly workaround. Hopefully someday we won't be restricted to Emscripten, and clients will be able to initialize the library themselves and pass the config object directly to the init function. The JS code is set up for that, but we're currently restricted to building with Emscripten, and its initialization order won't let us take advantage of that capability. Specifically: emscripten won't let us override the arguments accepted by the module init routine. Until it can, or we can use a different build tool, we have some unfortunate limitations imposed by Emscripten.

steida commented 7 months ago

But I'm using OPFS but not the one that needs those COOP, etc. headers. Are you telling me that SQLite WASM lib can not know which version of OPFS will be used? But I am explicitly saying that via installOpfsSAHPoolVfs call.

As for global config, thank you for your explanation; I wasn't aware of that.

Anyway, it's not a severe problem; it's just API DX. Thank you again for the docs update and your answers.

sgbeal commented 7 months ago

Are you telling me that SQLite WASM lib can not know which version of OPFS will be used?

Correct. They're two separate, independent VFSes, and the library cannot possibly know which of those you might want to use. They can both be used on the same web page, but cannot access the same databases. They each have separate trade-offs and the library cannot decide, for example, that "you have COOP/COEP, so you get this one, and that site does not have COOP/COEP, so they get the other one."