Avoid unnecessary async functions?

[oyooyo]

When looking at the source code, I came to realize that there are some functions that seem to be unnecessarily marked as being async.

The debug function in file dcc.js for example has the following implementation

export async function debug(uri) {
  return await decodeCbor(await unpack(uri));
}

So it is probably being marked as async because it uses and awaits the functions decodeCbor and unpack, which are both marked as being async.
But it seems to me that these two functions don't need to be async either; there is no await statement in the unpack function, and the only await statements in decodeCbor are when the decodeCbor function itself is being called recursively, so async/await should be unnecessary here as well.

I haven't looked at the rest of the code, so there might be more functions that are ultimately unnecessarily marked as being async.

[vitorpamplona]

Good catch!

I don't think we ever thought too much about when to declare something async AND the SDK main structure follows the 7 other SDKs we have and we want to keep their APIs similar. Thus, the decision to make something async should be based on the needs of the 7 SDKs, not only this one.

But I agree that we might have overdone with async calls.

[oyooyo]

I don't think we ever thought too much about when to declare something async AND the SDK main structure follows the 7 other SDKs we have and we want to keep their APIs similar. Thus, the decision to make something async should be based on the needs of the 7 SDKs, not only this one.

Ok, I guess that makes sense.

Two more tiny questions though:

1. Is there any kind of API documentation for this SDK yet?
2. How would you describe the current status of this SDK, is it already quite stable or rather an early alpha version?

[vitorpamplona]

Certainly, early alpha because the entire EU standard/proposal/code is early alpha. Most of the states are not fully compliant with the spec yet and even their main code changes frequently :)

[oyooyo]

Ok, thanks for the info.

One more remark:
In file dcc.js you may want to change the last line of function sign() from

return cose.create(headers, cborPayload, signer);

to

return await cose.create(headers, cborPayload, signer);

because cose.create() apparently returns a Promise. The code will probably work fine without the await, but I think adding the await makes it more obvious that cose.create() is asynchronous.

Btw, I noticed that the Copyright line at the beginning of some Javascript files says "PachCheck" not "PathCheck", I assume that's a typo.

[vitorpamplona]

Nice, thanks! I just released version 0.0.10, which should fix the cose.create call and the previous issue.

The code below was added. Let me know if your issues there were solved.

const main = require('./main.js');
module.exports = main;

[oyooyo]

As I just mentioned in the other issue, it works fine for me now. Thanks a lot for all your help.

One more thing: I noticed that when I run npm test, 13 of the tests are failing on my machine, both with the previous version and the current. Is that expected, or do all test cases run fine on your machine?

[vitorpamplona]

Yep, we are working on the tests. Unfortunately, that is a change on cose-js package to support the RSA keys in the way designed by the EU spec :(

[oyooyo]

FYI: I have converted the current version of the dcc-sdk.js SDK to a fully synchronous version, by including the required files from cose-js and adding synchronous variants of the required functions from that library.

I can comprehend that you probably do not want to pull these changes into this repository, as I can understand the reasoning of having the same structure for all SDKs. I basically just wanted to let you know that it is possible.

[vitorpamplona]

hum... why would you copy cose-js files into your branch? Is it just to remove the asyncs?

I might be missing something... is async that bad that you would prefer duplicating the code? Or is it creating an issue that you cannot go around?

I am very curious about it

[oyooyo]

Right now, cose-js seems to be exclusively async, at least there are currently no sync versions of the two functions being used by dcc-sdk.js, namely cose.create() and cose.verify(), even though there could be.
So I included the required files from cose-js and added sync versions of the required functions to them.

This is only meant to be a temporary workaround though, as I was looking for a quick solution.
Now that I know that's it's possible, I will try to convince the developers of code-js to also offer sync versions of functions that can be made sync. If that happens, I will remove the included files and instead use cose-js as a dependency again.

It's not that I hate async code or something, the easy handling of async code via async/await and Promises is in fact something I really love about modern Javascript.
But of course async does make some things a little more complicated. And since I didn't instantly see an obvious reason why dcc-sdk.js needs to be async, I was wondering if it wouldn't ultimately be less hassle to make dcc-sdk.js sync, instead of making everything that would depend on it async.

[oyooyo]

I just discovered that there actually already is a PR for introducing synchronous versions of cose.create() and cose.verify(), submitted only a few days ago.

So as soon as this gets merged, there is indeed no more reason to include a modified version of cose-js.