Error handling using GraphvizSync() layout

[magjac]

I'm not at all sure that this is a bug in hpcc-js/wasm, but I have indications that the error handling when using GraphvizSync() does not work. I'm getting an error saying "full function or function signature mismatch" instead of the expected "syntax error in line 1 near 'bad'".

I haven't yet had time to boil this down to a minimal test case, so for now this is mainly a question if you have tested this. Error handling using the asynchronous Graphviz() seems to work as expected.

More details can be found at https://github.com/magjac/d3-graphviz/actions/runs/3372738896/jobs/5596518645

If this seems to work for you I will try to carve out some time to create a minimal test case.

[GordonSmith]

@magjac - thanks, FWIW this was caused by a change in the latest c++ layer, where syntax error exceptions are no longer caught within the c++ code (and I had fixed it in the async variant).

[magjac]

Excellent! Thanks for the rapid fix. A new release would be immensely appreciated.

[GordonSmith]

@magjac that release is out now, with added Zstd compression wasm and base91 wasm...

[magjac]

Thank you so much. I however get a lot of failures in my tests with that version:

  ✔ data() returns the data saved by dot().
wasm streaming compile failed: TypeError [ERR_INVALID_ARG_TYPE]: The "source" argument must be an instance of Response or an Promise resolving to Response. Received an instance of Object
falling back to ArrayBuffer instantiation
Aborted(both async and sync fetching of the wasm failed)
failed to asynchronously prepare wasm: RuntimeError: Aborted(both async and sync fetching of the wasm failed). Build with -sASSERTIONS for more info.
Aborted(RuntimeError: Aborted(both async and sync fetching of the wasm failed). Build with -sASSERTIONS for more info.)
evalmachine.<anonymous>:936
  1) .destroy() deletes the Graphviz instance from the container element (shared worker version)
    var Module=typeof cpp!="undefined"?cpp:{};var readyPromiseResolve,readyPromiseReject;Module["ready"]=new Promise(function(resolve,reject)
...

I haven't had time to look into them yet so it might still be something I'm doing wrong, but I thought I might let you know directly if you recognize something.

It seems that it is only the tests that have problem. The build seems to work fine.

Full log:
https://github.com/magjac/d3-graphviz/actions/runs/3381171576/jobs/5614782415

[GordonSmith]

Is your test/@hpcc-js/wasm/dist/wrapper.js used in a NodeJS environment? If so it should be wrapping node_modules/@hpcc-js/wasm/dist/index.node.js (cjs) or node_modules/@hpcc-js/wasm/dist/index.es6.mjs (esm)

[GordonSmith]

dist/index.js -> browser UMD
dist/index.es6.js -> browser ESM
dist/index.node.js -> NodeJS CJS
dist/index.node.es6.mjs -> NodeJS ESM

I think your errors are from NodeJS loading a browser bundle...
Or it may be simply not locating the wasm file for reading (you may need to call wasmFolder)

[magjac]

Some feedback:

1. The error handling fix now works. Thanks a lot.
2. The wrapper is used inside a web worker implemented by https://github.com/avoidwork/tiny-worker. I don't know what kind of environment that is, but I just read someting about enabling ES6 in the docs. Will try another day.
3. node_modules/@hpcc-js/wasm/dist/index.es6.mjs does not exist. I tried with node_modules/@hpcc-js/wasm/dist/index.node.es6.mjs and node_modules/@hpcc-js/wasm/dist/index.es6.js, but none of them even loads in the web worker.
4. I then tried with node_modules/@hpcc-js/wasm/dist/index.node.js and it loads, but fails on Object.defineProperty(exports, '__esModule', { value: true }); since exports is undefined. I worked around that, by creating it before importing the script and then it came further, but failed the call: const __filename$1 = url.fileURLToPath(importUrl);. Then I decided I was too far into the rabbit hole and called it a day.

I will start with a fresh brain another day. Any tips are welcome. It seems that something radical changed in version 1.19.0, but it may very well be my usage that has been wrong the whole time.

[GordonSmith]

Sounds like I sent you down the wrong path - I thought the wrapper code was a helper for your NodeJS based unit tests and not for web workers in general... Let me sanity check with one of my other projects that uses web workers.

[magjac]

All of this is in NodeJS based tests. It works fine in the browser.

[GordonSmith]

@magjac - The issue is my end (missing "globalThis") fix is incoming. Also I added a simple webworker unit test, so will catch these in the future.

[GordonSmith]

@magjac - fix in 1.19.1 - thanks for spotting this!

[magjac]

Still no luck. I haven't had time to look into the details, but:

dist/index.js -> wasm error as per above
dist/index.es6.js -> loads, but fails somewhere
dist/index.node.js -> loads, but fails somewhere
dist/index.node.es6.mjs -> loads, but fails somewhere

Which one am I supposed to use inside the tiny-worker in NodeJS test?

In version 1.18.0 dist/index.js worked fine. Is it no longer supposed to work for my case?

[GordonSmith]

I added a NodeJS worker unit test (without any helper modules like tiny-worker) and its working fine.

dist/index.js isn't expected to work with ESM NodeJS, but with the new the "external" section in the package.json your environment is supposed to auto pick the correct variant.

FWIW You can see the diffs in the vanilla Browser and NodeJS workers (Base91 is another wasm file in @hpcc-js/wasm):

NodeJS:

import { parentPort } from "node:worker_threads";
import { Base91 } from "../index-node";

parentPort?.on("message", async function (data) {
    const base91 = await Base91.load();
    const base91Str = base91.encode(data);
    const data2 = base91.decode(base91Str);
    parentPort?.postMessage(data2);
    process.exit(0);
});

Browser:

import { Base91 } from "../index";

onmessage = async function (e) {
    const base91 = await Base91.load();
    const base91Str = base91.encode(e.data);
    const data = base91.decode(base91Str);
    postMessage(data);
};

[GordonSmith]

@magjac - you may want to hold off on this as I have a new build pipeline working (which has been in the works for a while):

1. Build wasm as normal
2. Compress wasm with zstd
3. Base 91 encode compressed wasm
4. Inline in JavaScript
5. Include JS libs to decode and decompress and initialise wasm

What you end up with is a single JS file including the wasm which will be NodeJS and Browser compatible and no need to worry about things like wasmFolder!

...and...

For GraphViz the entire JS file clocks in at 600k compared with the wasm file which is 1046k on its own.

[magjac]

@magjac out of curiosity, why does d3-graphviz need to support NodeJS environments anyway?

I don't know where this comment went, but the answer is that it doesn't. I only use NodeJS for test.

[GordonSmith]

IMO You might want to run your tests in a web browser and use something like karma to orchestrate those tests in a headless environment (like GitHub actions) - you are doing a "lot" of work in your test folder to fake a web environment in NodeJS by the looks of it?

I have updated my very simple index.html file to include samples of:

• Module loading
• Web worker
• Loading on demand.

Online demo: https://raw.githack.com/hpcc-systems/hpcc-js-wasm/trunk/index.html

For web workers, this is the html:

    <script type="module">
        const div = document.getElementById("placeholder5");
        const myWorker = new Worker("./index-worker.js", { type: 'module' });
        myWorker.onmessage = (e) => {
            div.innerHTML = e.data;
        }
        myWorker.postMessage(dot);
    </script>

And index-worker.js:

import { Graphviz } from "https://cdn.jsdelivr.net/npm/@hpcc-js/wasm/dist/sfx-graphviz.esm.js";

onmessage = async e => {
    const graphviz = await Graphviz.load();
    const svg = graphviz.layout(e.data, "svg", "dot");
    postMessage(svg);
}

Which is pretty clean. You can swap out the URL for "@hpcc-js/wasm/dist/sfx-graphviz.esm" or similar and bundle it with rollup to make it self contained...

[GordonSmith]

@magjac - Just published v2.0.0 which is now ESM by default and comes with a simplified load and use pattern (and new docs): https://hpcc-systems.github.io/hpcc-js-wasm/classes/graphviz.Graphviz.html

[magjac]

Sorry for the delay. Thanks a lot for your efforts.

It seems I have a lot of investigation/rework to do. Unfortunately I have almost no time for this in the very near future. Would it be possible for you to make a patch release on v1.18.0 with only a fix for this issue? This way I could make a new release of d3-graphviz and deal with these somewhat larger issues later on.

[GordonSmith]

Sure - let me know which branch was working for you last and I will base on that? I will do (based off v1.18.0)

[magjac]

Thank you so much. I really appreciate it.

Either you can fix this issue (#125) on v1.18.0 in which WASM loading works fine for me, or you can upgrade Graphviz in v1.16.1 which was the version that I used without any problems in d3-graphviz v4.4.0.

[GordonSmith]

@magjac Hopefully v1.16.6 should work...

[magjac]

Thanks. Unfortunately #120 bit me. I forgot that this caused issues in after having adopted type=module. Perhaps a patch on v1.18.0 is the only short-term solution for me after all.

I'm so sorry for the confusion.

[magjac]

For the record: @GordonSmith fixed my problem in magjac/d3-graphviz#254. Many many thanks, Gordon ❤️