Make ngrok errors more robust and helpful #1874
Conversation
|
For the incorrect auth token issue, here's with and without the change, for app and extension:
For the double-tunnel issue, I got ngrok running in a VM, and then tried these commands on my local machine, with and without the change, for app and extension.
|
| @@ -115,6 +115,8 @@ module Messages | |||
| tunnel_already_running: "A tunnel running on another port has been detected. Close the tunnel and try again.", | |||
| }, | |||
| tunnel: { | |||
| duplicate_session: "Another ngrok tunnel is currently running with your auth token, possibly on another machine. You must terminate that tunnel before opening a new one.\nAdditional information from ngrok logs: %s", | |||
| invalid_token: "The ngrok token you configured has expired. After generating a new token, you can update your local ngrok configuration using `shopify app tunnel auth <token>`\nAdditional information from ngrok logs: %s", | |||
There was a problem hiding this comment.
Are you sure the token can expire? I can't find anything on the docs about that: https://ngrok.com/docs#getting-started-authtoken
There was a problem hiding this comment.
Good point! I think you can only rotate it manually. So perhaps something like, "The ngrok token you configured has been invalidated."
lib/shopify_cli/tunnel.rb
Outdated
| @@ -178,6 +178,14 @@ def install(ctx) | |||
|
|
|||
| def fetch_url(ctx, log_path) | |||
| LogParser.new(log_path) | |||
| rescue NgrokError => e | |||
| case e.message | |||
| when /\AYour account is limited to 1 simultaneous ngrok client session/ | |||
There was a problem hiding this comment.
It'd be awesome if Ngrok used status codes when exiting their process unsuccessfully. I created this issue to keep track of these things to share feedback with them.
There was a problem hiding this comment.
@pepicrft I think they actually do use error codes. Sounds like I should edit this to look for the error code rather than the text, and maybe link to that doc in a comment here.
|
@amcaplan and I chatted today and I offered the principle of eliminating "You must" from error messages. So the string would be "Stop running the tunnel" or "Terminate the tunnel" rather than "You must terminate..." We also chatted about removing the logs so as not to expose the token if someone were to create a GitHub issue. |
|
This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action. → If there's no activity within a week, then a bot will automatically close this. Thanks for helping to improve Shopify's dev tooling and experience. |
We see many ngrok errors in bugsnag. This will make it easier to understand what's happening and take action.
* Don't print logs which may include sensitive information * The token is invalid, not expired. ngrok tokens don't expire. * No "you"
amcaplan
changed the title
amcaplan
force-pushed
the
gentler-ngrok-dual-tunnel-error-handling
branch
from
68242af
to
de0a40d
Compare
amcaplan
requested review from
a team,
jesalerno84 and
molly-yu
and removed request for
a team
|
Hehe, forgot about this until the stale bot reminder. Updated and ready to merge! |
amcaplan
force-pushed
the
gentler-ngrok-dual-tunnel-error-handling
branch
from
08c94da
to
4e84cec
Compare
WHY are these changes introduced?
We see many ngrok errors in bugsnag. This will make it easier to
understand what's happening and take action.
WHAT is this pull request doing?
Catching errors frequently observed in bugsnag and giving more helpful feedback.
How to test your changes?
For the dual tunnel issue, I think you might need to have 2 ngrok sessions on 2 different machines. Docker might help here.
Post-release steps
Update checklist