refactor: revert back to *old* way of cloning

[danielpeintner]

Note: reverts parts of #1105

fixes #1151

[danielpeintner]

The issue with starting node packages\cli\dist\cli.js examples\scripts\counter.js is gone but the exposed thing immediately shuts down and the "server" part is not active anymore...

[danielpeintner]

We run into the case where no server is there...

node-wot/packages/core/src/servient.ts

Lines 39 to 44 in 8659709

	if (this.servers.length === 0) {
	warn(`Servient has no servers to expose Things`);
	return new Promise<void>((resolve) => {
	resolve();
	});
	}

2 questions:

• why does this happen?
• why is the warn statement not printed out ?

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (1441a01) 75.17% compared to head (2976374) 76.60%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1152      +/-   ##
==========================================
+ Coverage   75.17%   76.60%   +1.42%     
==========================================
  Files          80       80              
  Lines       16576    16578       +2     
  Branches     1592     1596       +4     
==========================================
+ Hits        12461    12699     +238     
+ Misses       4067     3852     -215     
+ Partials       48       27      -21     

Files	Coverage Δ
packages/core/src/exposed-thing.ts	72.27% <100.00%> (+4.73%)	⬆️
packages/cli/src/cli-default-servient.ts	70.73% <0.00%> (ø)

... and 20 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[danielpeintner]

I found the problem, see 2976374

Question: I wonder whether the clientOnly flag is properly initialized...

https://github.com/eclipse-thingweb/node-wot/pull/1152/files#diff-d50fb68661fd819c0149ceef494c0015de6739c57acd49363e2ba04d6aa0eda3L99-L102

I would rather write something like..

        // apply flags
        this.config.servient ??= {};
        this.config.servient.clientOnly = clientOnly;

Any thoughts?

[JKRhb]

Thank you, @danielpeintner, for providing this fix! After also doing some more research, I discovered that the problem might be related to vm2, since the error does not occur if you perform a structuredClone of an ExposedThingInit before passing it to the produce method (which I found pretty odd). Apparently, vm2 wraps unprocessed JavaScript objects somehow, so that they are not compatible with structuredClone anymore. With the old parse-stringify approach, this problem does not seem to be present.

I guess we could revisit using the more recommended structuredClone once again when a replacement for vm2 has been added to the code. For now, maybe we need to make sure that the problem does not appear in other CLI-related contexts as well.

I am really sorry that this problem arose, I didn't anticipate this kind of behavior from the CLI :/ Really great that you discovered the solution so quickly, though!

[JKRhb]

Question: I wonder whether the clientOnly flag is properly initialed...

https://github.com/eclipse-thingweb/node-wot/pull/1152/files#diff-d50fb68661fd819c0149ceef494c0015de6739c57acd49363e2ba04d6aa0eda3L99-L102

I would rather write something like..

        // apply flags
        this.config.servient ??= {};
        this.config.servient.clientOnly = clientOnly;

Any thoughts?

That is definitely a much simpler solution and looks good to me! I think the current state resulted to a certain extent from the version we had before where the servient field was not always defined (or only when clientOnly was "truthy"). Maybe we need to double-check that this slight logic change would still work but other than that I am very much in favor of this change.

[danielpeintner]

That is definitely a much simpler solution and looks good to me!

@relu91 are you fine with the suggested change as part of this PR also?

[relu91]

Yes good to go, and thanks for digging this down. VM2 is getting problematic :/