Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect if op fails to talk to the 1Password desktop app #209

Open
zcutlip opened this issue Dec 18, 2024 · 5 comments
Open

Detect if op fails to talk to the 1Password desktop app #209

zcutlip opened this issue Dec 18, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@zcutlip
Copy link
Owner

zcutlip commented Dec 18, 2024

Under various conditions op will fail if expects to talk to 1Password but it can't. This can happen if the 1Password app has been terminated or died.

It also appears to happen if op has DOSed the app and it can't respond fast enough[1].

We should detect this situation and raise a more specific exception than OPCmdFailedException so the caller can decide what to do.

[1] reported by @jchan-legendpower in #137

@zcutlip
Copy link
Owner Author

zcutlip commented Dec 18, 2024

We'll need to scrape an error message. It should be something like:

[ERROR] 2024/12/18 15:53:45 connecting to desktop app: 1Password CLI couldn't connect to the 1Password desktop app <truncated>

@zcutlip
Copy link
Owner Author

zcutlip commented Jan 17, 2025

I've seen two different but similar errors:

[ERROR] 2025/01/16 19:56:31 connecting to desktop app: connecting to desktop app timed out, make sure it is installed, running and CLI integration is enabled

and:

[ERROR] 2025/01/16 19:56:32 connecting to desktop app: 1Password CLI couldn't connect to the 1Password desktop app.

When I do the equivalent of kill $(pgrep 1Password), then do op=OP(), I get the first error, then if I try again, I get the second error. On the third try, it succeeds, 1Password having been restarted, presumably by launchd.

One theory is that in the first attempt 1Password is still shutting down, and on the second attempt it is already gone down. Perhaps the IPC (XPC/mach in my case since I'm on macOS) behaves differently in the two cases.

In any case, what this means for this issue is that when OPDesktopAppException is raised, it may be prudent for the caller to try two more times, sleeping a short period between tries.

@zcutlip
Copy link
Owner Author

zcutlip commented Jan 17, 2025

@jchan-legendpower apologies this has taken a while.

I have a working solution in the dev/209-detect-op-desktop-app-failure branch.

I've detected connection failure with the desktop app, and now raise OPDesktopAppException

There's an example under examples/ also in this branch: https://github.com/zcutlip/pyonepassword/blob/dev/209-detect-op-desktop-app-failure/examples/desktop-app-exception.py

The example first kills all 1Password-related processes, then attempts to sign in with op=OP(), handling the exception if it fails.

@jchan-legendpower
Copy link

Is there a need to kill everything and reauth? I've been using my polling workaround and it seems to always eventually succeed, would rather not incur a second auth confirmation.

Behaviour might also be different between Windows and MacOS

@zcutlip
Copy link
Owner Author

zcutlip commented Jan 17, 2025

Is there a need to kill everything and reauth

Absolutely not! Sorry I wasn't clear...killing all 1Password process in the example script was just to simulate 1Password dying or becoming unresponsive. This forces a connection failure with the desktop app in order to demonstrate the exception being raised.

I don't mean for the user to actually kill 1Password.

I'll add a clarifying comment to the example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants