Synchronous methods

[wbrickner]

Summary

I'd like to be able to use all the public/top-level methods of keytar synchronously, but most urgently the getPassword function.

Motivation

My project stores small bits of sensitive application state using keytar. These bits of data are loaded on application launch to restore those sensitive portions of the state.

The fact that all methods are async means that entire modules that utilize keytar need to be wrapped in async functions themselves, or need special machinery to dynamically create their own functionality after data from keytar becomes available. Trust me, all of this is an unavoidable mess in my use-case.

Having optional sync methods would fix this problem and make life easier.

Describe alternatives you've considered

I'm aware of async/await for making the code look synchronous, but this doesn't cut it in my case, and does nothing to avoid the real async nature of the keytar methods.

Additional context

For what it's worth, I'm fond of appending Sync to function names:

getPasswordSync(service, account)
setPasswordSync(service, account, password)
deletePasswordSync(service, account)
findCredentialsSync(service)
findPasswordSync(service)

If you can point me in the right direction, I can try to implement this myself, but I have little experience with node add-ons, node-gyp, etc.

Thank you!

[shiftkey]

Cross-referencing this earlier comment which is still how I feel about this https://github.com/atom/node-keytar/pull/143#issuecomment-453762037

---

But my question still remains should not we allow programmers to choose between sync and async functions if we can.

I found this even older comment asking why the APIs weren't asynchronous https://github.com/atom/node-keytar/issues/58, and pointed to the synchronous APIs actually being slower at the time:

They tend to be slower (around ~40ms on my 2015 MacBook Pro) and seems like there could be some benefit to making them non-blocking for performance reasons (especially as it's probably best to run node-keytar methods in the main process).

Node itself is built on technologies like libuv, and synchronous APIs can actually be slower due to how the runtime works.

That comment was cited in the work ages ago to move from synchronous to asynchronous APIs in https://github.com/atom/node-keytar/pull/63 to provide consistency across the modules that the Electron and Atom teams maintain, and I've not seen much feedback in terms of missing the old synchronous APIs since then.

As I know performance could be a factor when you run this code on server, when we run code in sync on server, but this keytar is also used with electron i.e. mainly on computer, we can fork this repository and make keytar-electron if we do not want changes on because it could slow down the server code, if not done properly on server.

You are also welcome to fork this and maintain your own module which contains synchronous APIs. Electron apps make up a big part of keytar usage currently, and I want to continue to ensure they have first-class support here.

Also, can somebody explain what would be other cons of having sync function in the code base please.

Some things that come to mind:

• Maintenance (implementing and maintaining two sets code paths)
• More support needed (because the API surface is now doubled)
• Behaviour differences between sync and async internals (returning results, propagating errors, etc)
• Performance unknowns (the asynchronous APIs are built upon libuv primitives, and it's not clear to me what the plan is for the sync APIs)

[wbrickner]

I'm not sure I understand the performance argument. Of course you shouldn't be using sync methods on-the-fly in a server or desktop application, but at startup time that small performance penalty can make a lot of sense if it dramatically simplifies the structure of your application.

This is the same as how built-in modules like fs provide a synchronous interface, and it's widely acknowledged that this is valuable but shouldn't be blindly used as a replacement for the async methods.

I hear the complaint about maintaining multiple code paths. I've opened a pull request on this repository containing an implementation for sync methods with the tests and documentation already written. I don't think it's a huge inflation of the codebase, but maybe at this scale those small changes lead to headaches later that I'm not thinking of.

The behavior and code between the sync and async methods (in my implementation) is almost identical, so there isn't a lot of conceptual complexity there, but there is more redundancy and it can be made more abstract.

I've done no performance testing on the sync APIs, but the async API performance should remain identical because it's not been modified.

Just to be overly clear, downstream consumers of this module would be unaffected and probably unaware that these sync methods have been made available (in terms of module size or runtime performance).

I've made my changes in a forked repository, and anyone who needs the sync methods can use them today by using my repo. (https://github.com/wbrickner/node-keytar)

[mzabaluev]

Somewhat ironically, keytar calls synchronous functions in libsecret under the hood.

I must admit though that integrating asynchronous event loops of node and GLib is not for the faint of heart, having spent some time on it myself: https://gitlab.gnome.org/GNOME/glib/-/issues/693

[itfilip]

+1 for adding sync methods in Keytar.

I'm using Keytar in an Electron application. I need to load some keys during application startup. Async methods dramatically increases the complexity of my application and in some cases it is impossible to implement without rewriting a bunch of code. I tried keytar-extra. It works well with older versions of electron and electron-rebuild, but for some reason I'm not able to rebuild it using the latest electron and electron-rebuild. Please, if possible, merge keytar-extra into keytar and keep it updated.

[wbrickner]

I patched the repo 2 years ago to expose sync methods. Turns out it's really simple.

It's quite far behind now, but it works just the same.