search

alondmnt / joplin-plugin-jarvis

Joplin (note-taking) assistant running a very intelligent system (OpenAI/GPT, Hugging Face, Gemini, Llama, Universal Sentence Encoder, etc.)
GNU Affero General Public License v3.0
226 stars 22 forks source link

Question: Jarvis throwing errors in development tools #28

Closed djsudduth closed 4 months ago

djsudduth commented 4 months ago

When Jarvis is enabled, it keeps throwing this error in the Joplin dev tools:

models/Setting: Could not set setting on the keychain. Will be saved to database instead: plugin-joplin.plugin.alondmnt.jarvis.springer_api_key: Error: Password is required.

Jarvis is working fine with the API key for GPT - so, I don't know why it's throwing this error over and over

Joplin 2.14.22 (prod, win32) Client ID: 19f20674be77445bbd5535190049d93c Sync Version: 3 Profile Version: 46 Keychain Supported: Yes Revision: e579eb9

alondmnt commented 4 months ago

I believe it's an internal Joplin error. Due to the fact that API Keys are considered hidden / secure fields, Joplin tries to store them in the keychain, and it falls back to using its database. I'm not sure it's reason for concern, but maybe Joplin devs will know better.

djsudduth commented 4 months ago

@alondmnt so any secure field in a plugin causes this and is attempting to use the keystore? When I disable Jarvis there are no errors.

alondmnt commented 4 months ago

I'm using this property in the plugin API for Jarvis API keys (there are 5 such fields in Jarvis' settings, for example).

I'm attaching a version of the plugin where I only changed the secure property to false. Everything else is the same. Could you check if this removes the exception from the log? (I wasn't able to reproduce it on my end, but I've seen this error in the past.)

joplin.plugin.alondmnt.jarvis.jpl.zip

Also, if you're willing to provide more info on your OS and version, maybe we can figure out why the keychain service is unavailable in your case.

djsudduth commented 4 months ago

@alondmnt that removed the error. I'm not sure why the keystore isn't working. I have Python applications that use the keystore with the Python keychain modules successfully (Windows 10 Pro - 10.0.19045).

I switched back to your current released version - here is the full error output:

models/Setting: Could not set setting on the keychain. Will be saved to database instead: plugin-joplin.plugin.alondmnt.jarvis.hf_api_key: Error: Password is required. at checkRequired (C:\Users\myusername\AppD…\lib\keytar.js:5:11) at Object.setPassword (C:\Users\myusername\AppD…\lib\keytar.js:20:5) at KeychainServiceDriver. (C:\Users\myusername\AppD…river.node.js:19:43) at Generator.next () at C:\Users\myusername\AppD…Driver.node.js:8:71 at new Promise () at __awaiter (C:\Users\myusername\AppD…Driver.node.js:4:12) at KeychainServiceDriver.setPassword (C:\Users\myusername\AppD…river.node.js:16:16) at KeychainService. (C:\Users\myusername\AppD…ainService.js:51:32) at Generator.next ()

It feels like an internal Joplin error of the KeychainService.js. I did see some possible issues online on where KeychainService is executed within the wrong Electron layers.

Are you going to submit this to Joplin issues?

djsudduth commented 4 months ago

PS - I see Joplin is using keytar 7.9.0 as the keychain library. I don't like seeing that the repo is now archived - https://www.npmjs.com/package/keytar

alondmnt commented 4 months ago

Thanks, submitted an issue. You are welcome to follow and add to the discussion there.

djsudduth commented 4 months ago

Problem discovered in the keytar library. See the issue here - > https://github.com/laurent22/joplin/issues/10526#issuecomment-2144169722

Short term Jarvis fix is needed in settings.ts -> register_settings() needs to either ignore any secure settings if empty or save a special Jarvis token placeholder in it and replace it as empty on a read if the user field is blank.

Hopefully Joplin core will fix this (and replace keytar)

Hope this helped!

alondmnt commented 4 months ago

Strange, I have some empty secure fields and it still works, but I'm willing to work on a fix. I'll upload it here when it's ready. (BTW, there are other advantages to initializing with dummy API keys, so why not.)

djsudduth commented 4 months ago

I don't think it causes any direct end user errors other than a lot of logging errors (1 for each empty secure field every 10-20 seconds). So, not really something users would notice. Might eventually cause performance issues - but not sure.

Thx for the great plugin!

alondmnt commented 4 months ago

I meant that I don't see errors in my logs despite having some empty fields.

In any case, I published a new release that should fix this. You may see the error once when first installing the plugin, but the default values should be set and resolve this.

Thanks for looking into this!