API version may be set only once

[ex3ndr]

We are constantly hitting this error while initing our stack. Right now it blocks us from releasing next version of @openland/foundationdb. it seems there are some race conditions on native side and we are successfully using patch-package to wrap this error in library and just ignore it.

[ex3ndr]

Can't wrap it outside of the library since then it will throw exception about missing api version.

[josephg]

Hm - it should work without throwing if you set the API version to the same number in multiple places from your code.

If you're using different API versions from different places in your code, the semantics of a few functions can subtly change, which might introduce bugs (now or in the future). But if you're not worried about that it should be ok to put a try-catch around the method call. And I get that the current model breaks the normal nodejs model for versioning - I'm tempted to hardcode an API version and use that, but that puts a lower bound on the version of your backend database, which would break the library for some other users. (Eg you can't use API version 610 with database version 6.0.x). I guess another approach would be to just automatically use API version 600 or so if you don't specify a version anywhere - which is more complicated but probably fine in the 95% use case.

I'm not sure how else to fix this - the semantics of that function come from the C API. There's probably an approach involving dlopen-ing the library file so we can use multiple API versions at the same time, but thats quite complicated in OS-specific ways and I'd rather not do that.

I guess I'm trying to ask, what would you like the behaviour to be here?

[ex3ndr]

No. It is the very same version. I am not sure what is expected, but our patch is

patch-package
--- a/node_modules/foundationdb/dist/lib/apiVersion.js
+++ b/node_modules/foundationdb/dist/lib/apiVersion.js
@@ -31,10 +31,16 @@ https://github.com/josephg/node-foundationdb/issues
 Until this is fixed, use FDB API version ${exports.MAX_VERSION}.
 `);
         }
-        if (headerVersion == null)
-            native_1.default.setAPIVersion(version);
-        else
-            native_1.default.setAPIVersionImpl(version, headerVersion);
+        try {
+            if (headerVersion == null)
+                native_1.default.setAPIVersion(version);
+            else
+                native_1.default.setAPIVersionImpl(version, headerVersion);
+        } catch (e) {
+            if (!e || e.message !== 'API version may be set only once') {
+                throw e;
+            }
+        }
         apiVersion = version;
     }
 }

This fix works just fine. We are hitting problems mostly in jest at very same place, i think native code just started only once while V8 started several times for some reason.

[ex3ndr]

I believe that we need to put same check on native side here: https://github.com/josephg/node-foundationdb/blob/master/src/napi/module.cpp#L53

Unfortunately i have very little experience with native libraries for nodejs.

[josephg]

Ah well then; its just a bug. Because the native code is currently duplicated, this is easier to solve from javascript. We just need to cache the api version (and header version I think) the first time setAPIVersion is called in that file. If the function is called again with the same arguments, the function should return immediately without doing any work.

If you put together a PR I'll merge it.

[ex3ndr]

No, there is already cache or at least flag that version was set. But it is false. Means js part was never called before.

[josephg]

Huh? Then color me confused. How is the version already set if not via the JS code? I assumed if multiple versions of this library were pulled in via npm they’d each end up with separately scoped libfdb_c files but maybe that’s not true...

Is that what’s happening?

[ex3ndr]

Jest creates new js execution contexts for each test, but native libraries are not reloaded for each context.

From JS perspective setAPIVersion was never called, but from native one it was.

[ex3ndr]

I have done PR #31 to fix this issue

[josephg]

Hm ok. That’s ... surprising to me. I’m not convinced that making this change in the native part of the module will help either. I suspect the native module might be reloaded but it might keep libfdb open from dlopen. I can make a PR you can use to test it - can you tell me the exact version of nodejs you’re using?

[josephg]

..Ah cool you did it. Can you verify your change fixes the issue you're seeing?

[ex3ndr]

I am using v10.16.0.

[ex3ndr]

I am not sure, it wasn't easy to use my fork. I checkout my branch, copied to my project (just including via git didnt work because of jest typings conflict) and tried to start my tests. Now everything works just fine. Reverting my changes will cause a crash, yes.

[ex3ndr]

BTW, you can try to check problem by yourself: https://github.com/openland/foundationdb/commit/87290eab16cca203b45fc9994239902fe4c9e9c0

Just checkout, yarn && yarn test, you will see an error. My branch solves this issue.

[josephg]

Awesome :)

I'll take a look at the PR and we can get it merged. We should also copy the functionality into the nan implementation of the native module - its sort of awful right now; that code will eventually be deleted (maybe at the end of the year or something like that).

[ex3ndr]

Do you have any timeline for this? This is blocker for us.

[josephg]

No sorry; timelines are anti-fun for me so I don't supply them when I'm volunteering my time on evenings and weekends. I only provide timelines & estimates to clients I have a commercial relationship with.

Once github sponsorships are released widely you'll have the opportunity to sponsor the project & get much clearer estimates for things like this.

[josephg]

Ping - are you going to finish this PR? I'd like to merge it! We need to merge across the change into the nan directory or the change won't have any affect on your version of nodejs after we publish.

[ex3ndr]

Ah, that's why it is not merged yet. Will update everything shortly.

[ex3ndr]

@josephg Done.

[josephg]

Cool I'll take a look soon

[josephg]

📦foundationdb@0.10.3