-
Notifications
You must be signed in to change notification settings - Fork 845
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
Show specific error messages on errors creating the tunnel. Fixes #972 #1302
base: master
Are you sure you want to change the base?
Show specific error messages on errors creating the tunnel. Fixes #972 #1302
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
I used old output from |
3d7cf85
to
962b8fb
Compare
I removed the commit containing secret-looking test data and now GitGuardian is happy. |
@DevinCarr Hi Devin - you seem to have been active in this part of cloudflared recently. Could I get a review on this PR? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this, I left a small comment, but looks good otherwise
Hi @metadaddy, thank you for making this contribution. We made a change to the quick tunnel file recently. Can you rebase? |
962b8fb
to
6b8e0e2
Compare
@chungthuang Rebased and force-pushed. @GoncaloGarcia Apologes - I missed your comment when you sent it. I added the HTTP response body to the error message as you requested. |
6b8e0e2
to
d3a6ed9
Compare
@chungthuang / @GoncaloGarcia / @kornelski - I noticed that |
As described in #972, when creating a tunnel,
cloudflared
shows a somewhat cryptic error message on HTTP errors such as 429 (too many requests), sinceRunQuickTunnel()
tries to parse the HTTP response body as JSON no matter the response status code.This change detects non-200 status codes, shows a specific error for 429, and reports the status code for other HTTP errors.
Tests are included, and all existing and new tests pass.