search

dblock / strava-ruby-cli

Strava command-line client.
MIT License
5 stars 1 forks source link

Ensure error if the browser can't launch #3

Closed dblock closed 4 years ago

dblock commented 4 years ago

Looks like if the browser can't launch there's no error.

For the auth workflow with the cli we want to terminate the local server and fail with a clear error if that's the case.

mattdsteele commented 4 years ago

As an example, in a WSL environment without the BROWSER env variable set:

❯ ./bin/strava console
/var/lib/gems/2.5.0/gems/launchy-2.5.0/lib/launchy.rb:69: warning: Insecure world writable dir /home/matt/.vscode-server/bin in PATH, mode 040777
[2020-06-01 07:51:38] INFO  WEBrick 1.4.2
[2020-06-01 07:51:38] INFO  ruby 2.5.5 (2019-03-15) [x86_64-linux-gnu]
Enter Strava client ID: XXXXX
Enter Strava client secret: *****************************************
xprop:  unable to open display ''
xprop:  unable to open display ''
[2020-06-01 07:51:52] INFO  WEBrick::HTTPServer#start: pid=11396 port=4242
/usr/bin/xdg-open: 870: /usr/bin/xdg-open: www-browser: not found
/usr/bin/xdg-open: 870: /usr/bin/xdg-open: links2: not found
/usr/bin/xdg-open: 870: /usr/bin/xdg-open: elinks: not found
/usr/bin/xdg-open: 870: /usr/bin/xdg-open: links: not found
/usr/bin/xdg-open: 870: /usr/bin/xdg-open: lynx: not found
/usr/bin/xdg-open: 870: /usr/bin/xdg-open: w3m: not found
xdg-open: no method available for opening 'https://www.strava.com/oauth/authorize?approval_prompt=auto&client_id=XXXXX&redirect_uri=http%3A%2F%2Flocalhost%3A4242%2F&response_type=code&scope=read_all'

Then it hangs. I'll see if this can be caught from Launchy's side

mattdsteele commented 4 years ago

Launchy does have a block for catching exceptions, but at least in the case of WSL it does not appear to execute. I've opened a separate Issue on their repo: https://github.com/copiousfreetime/launchy/issues/136

Do you think it's worth adding the block regardless? I'm not sure if there are other cases beyond mine where it would be useful.

dblock commented 4 years ago

If this fixes some scenario, even if it's not 100%, then it's definitely worth it.