Better error message when initAztecJs is not called

[spalladino]

If a user forgets to call initAztecJs, the error message they get is Initialise first via initSingleton, which is not referenced in our docs or aztecjs API. We should throw an error that references initAztecJs instead.

Consider also removing the need for calling initAztecJs explicitly, and lazy loading instead when it's actually used.

[charlielye]

We can't lazy load it as it's async, and that has massive async propagation that seems an unacceptable tradeoff.
I'm happy to reconsider making it auto init on module load (what we did before).
In fact now we have the "granular api" i think we can have just init when one imports something that actually needs it, plus combining that with dynamic imports means one can still control when the hit is taken.

My bad on not updating the docs around this...

[spalladino]

We can't lazy load it as it's async, and that has massive async propagation that seems an unacceptable tradeoff.
I'm happy to reconsider making it auto init on module load (what we did before).

I understand that before we did lazy load, which made every method that depended on the singleton async. I take it it's unacceptable because of developer convenience? I don't think the performance hit is significant.

But assuming the asyncs are unacceptable, then +1 to auto init. I'm curious though: what happens in cjs, since it doesn't like top-level awaits? I saw a ts-ignore related to it, but I don't know what actually happens if you try running it when in cjs.

[charlielye]

IIRC performance hit can become significant if you're e.g. having to do lots and lots of hashing in a short amount of time (tree computations etc). I guess this could be alleviated though by lazy fetching the wasm earlier in the stack.

Maybe I just can't get past how much more verbose the code becomes, and how otherwise logically synchronous functions all have to become async and incur event queue overhead because of an implementation detail.

Good point on CJS. We can't auto-init in cjs land. So you're forced to either async everything, have an init function, or not support CJS.
One could have it auto init in esm, and require init in cjs, but - seems like it might just be better to document.
It's not an issue at present as my "test hack" works because we run tests as esm.
Maybe CJS will just die in a year or so.

Hmmph. I drew great satisfaction in de-asyncifying several dozen functions. Maybe was mistake...

[charlielye]

I'm not even sure how to improve the error as it's thrown from bb.js but could get emitted by any method that leads to bb.js, so it can't be captured and transformed (easily).

edit: Well ok, there's about 10 places we call getSingleton so it could be done.

edit2: Can I just improve the error message and add initAztecJs to docs? I can't face asyncifying everything right now.

[spalladino]

Maybe CJS will just die in a year or so.

I hate it too, but I believe it's needed for the VSCode extension.

[spalladino]

Can I just improve the error message and add initAztecJs to docs? I can't face asyncifying everything right now.

+1. That was the original spirit of this issue.

[charlielye]

Just realized that to transform the error (properly), it has to be done in the aztec.js project, so somewhere within whichever of the many functions we export there, they could throw, they need to be wrapped etc.

Is there some pattern whereby we can make any use of aztec.js fail with a good error, if init hasn't been called? 🤔

[charlielye]

By leveraging more granular imports (and fixing some import cycles), I'm able to run "cli help" without it triggering a "top level await" init of bb.js. (And got it down to 0.095s to just display the help!)
I think this is a solution I'm most comfortable with (although it'll be easy to regress if we are not considerate of import dependencies, importing parents that import everything else, etc). The lazy init approach with async propagation, feels like a lazy approach (pun intended) that works around the fact we have this "import all the things pattern". It's going to take a bit of time and effort to construct a clean modular public api, but I think we can do it over time. That should mean you don't actually pay the cost of initing bb.js, if you don't use code that uses it (or is very close to code that uses it). This applies more generally beyond bb.js as well, e.g. just paying the costs of compiling modules.

I'm coming across the dreaded utils pattern. e.g. There's some util function that checks the length of something is something else. And it's in random catch all util file that therefore imports some other module (ungranular) that imports everything else etc etc. So to check the length of a buffer, we end up importing the entire lib. I generally think "utils" is a dangerous pattern that should be pushed back on. If we want to check the length of a buffer, just do the check inline. I get that the idea is to help display a nice error message, but in all probability, most engineers probably don't even know this function exists and won't use it. Most util functions end up a dependency nightmare and half used. They are usually "one engineers helper function".

If we want such functions, I would group array level "utils" into a module in foundation called "array", and ensure that module didn't import anything else irrelevant to it. In this particular case that was causing bb.js to init, I just inlined the buffer check in TxHash so it didn't have to import the circuits.js mega import that imports everything else. (There were other things I've had to solve, like circular imports to top level mega imports).

  constructor(
    /**
     * The buffer containing the hash.
     */
    public buffer: Buffer,
  ) {
    if (buffer.length !== TxHash.SIZE) {
      throw new Error(`Expected buffer to have length ${TxHash.SIZE} but was ${buffer.length}`);
    }
  }

Anyway, r.e. the approach of granular imports with top level init. This does mean however, that for the CJS path, the developer will have to remember to init the library at an appropriate point in time, as there is no top level await (and that case will need documentation). I think this is an acceptable deviation however, as we should generally hold the view that CJS will die, and that users of it can jump through the additional hoop of calling an init function until it does.

This is PR so far. Happy to discuss further. #3629

[spalladino]

Is there some pattern whereby we can make any use of aztec.js fail with a good error, if init hasn't been called? 🤔

Not that I can think of. We could wrap each function of aztec.js in another function that does the try/catch or play with decorators (since they're meant for this kind of stuff!), but that requires tweaking each function in aztec.js that potentially depends on bb.init. Another option could be iterating over all module.exports and tweaking them, but I don't like it either.

Honestly, with the improvement for esm, and the better error message in bb.js itself (that points the user in a right direction), I'd close this issue as fixed for now.