add timeout to clipboard initialization

[jonZlotnik]

TL;DR

• cli_clipboard hangs on initialization in some weird x11 setups (e.g. my WSL2)
• In linux, cli_clipboard depends on a connection to a socket.
• the clipboard functionality isn't critical, so we shouldn't fail if not working

Context:
I wanted to try my hand at the sending text feature,
so I tried getting this repo setup on WSL2, but I couldn't get my binary to run. It kept hanging with no output.

Into debug mode I went:
All I got was [2023-03-06T23:58:31Z DEBUG wormhole_rs] Logging enabled.

Now to debug Rust for the first time:
I found that it was stuck inside the cli_clipboard ClipboardContext::new() function.
And noticed that for Linux environments, it depends on an x11 socket.

There doesn't seem to be any kind of test suite for the cli at the moment, so let me know if there something I should add.

Note: I'm brand spanking new to Rust. So please be verbose with comments and teachings. 😅

[jonZlotnik]

1 second is probably over-cautious, if anybody can defend a shorter timeout, pls do. (I didn't do any research into how long it takes for cli_clipboard to init normally)

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.17 ⚠️

Comparison is base (a82a7ae) 43.27% compared to head (79ebb44) 43.10%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #185      +/-   ##
==========================================
- Coverage   43.27%   43.10%   -0.17%     
==========================================
  Files          18       18              
  Lines        2542     2552      +10     
==========================================
  Hits         1100     1100              
- Misses       1442     1452      +10     

Impacted Files	Coverage Δ
cli/src/main.rs	0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jonZlotnik]

Note: I'm actually having trouble replicating the issue I faced in other environments (even other WSL2 envs).
Nonetheless, blocking the main thread of any program on waiting for an "networky" OS resource without a timeout is usually not a good idea.

[piegamesde]

I'll try to have a look at it in the next couple of days. In the mean time, would you mind reporting the bug upstream to the relevant crate? Maybe they have some deadlock or other bug that can be fixed for that platform.

[jonZlotnik]

Issues seem to already be stacking up. Maybe switching to 1Password's arboard https://github.com/1Password/arboard could be good if they're not resolved timely?

cli_clipboard
allie-wake-up/cli-clipboard#16
allie-wake-up/cli-clipboard#17

x11_clipboard
quininer/x11-clipboard#32

@piegamesde you also added this and it's still open
allie-wake-up/cli-clipboard#10

[piegamesde]

Thanks for digging into this. I'm fine if you want to make a switch to arboard, the only requirement I have is that it has native Wayland support. Using it would have the added benefit of not depending on any native X libraries, as we currently do.

[piegamesde]

Finally had a look at the code. It looks okay, but I think it can be simplified: since we always only send one value, you can simply have it as return value of the closure and not use a channel at all. On the main thread, one would then join the clipboard thread with a timeout. If it succeeds print the value, otherwise kill the thread and print an error message.

Of course you could also try out arboard, which will probably make the background thread obsolete.

[jonZlotnik]

Okay, so I think you're correct in that the timeout fix should be upstream, but the cli_clipoard is too far downstream (depends on x11_clipboard, which depends on x11rb).

So I tried switching to arboard, but it actually has the same issue. Thankfully, they depend on x11rb directly, and a small fix is possible. So I submitted a PR there, and we'll see how it progresses.

1Password/arboard#100

If it goes through, I'll PR the switch to arboard.

[jonZlotnik]

Finally had a look at the code. It looks okay, but I think it can be simplified: since we always only send one value, you can simply have it as return value of the closure and not use a channel at all. On the main thread, one would then join the clipboard thread with a timeout. If it succeeds print the value, otherwise kill the thread and print an error message.

Of course you could also try out arboard, which will probably make the background thread obsolete.

I actually don't think it's possible without a channel unless there's some API I missed for joining threads. (very well might be)
Don't want to wait the full time if the thread returns successfully.

[jonZlotnik]

Thanks for digging into this. I'm fine if you want to make a switch to arboard, the only requirement I have is that it has native Wayland support. Using it would have the added benefit of not depending on any native X libraries, as we currently do.

Alright, so my PR finally got merged in 1Password/arboard#100

There seems to be the option to have Wayland by default:
https://github.com/1Password/arboard/blob/3399a2582f33085f9602b1acfaaa5847face8373/README.md?plain=1#L21-L27

[piegamesde]

Okay, how to proceed with this? Do you want to try switching to arboard?

[jonZlotnik]

Okay, how to proceed with this? Do you want to try switching to arboard?

Yeah I'll PR the switch to arboard. Sorry for the lapse.

[jonZlotnik]

Switch is PR'd in #203