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

httpError: write error string as body in 502 response #448

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

omribahumi
Copy link
Contributor

When CONNECT fails, other than getting a 502 response, there's no indication of the failure reason.

This pull request will:

  1. Print the dial failure reason to the Go log output
  2. Return the error in the 502 response body

I couldn't test this with curl as it suppresses the body for 5xx responses for some reason. Not sure if this is because this is a 502 in response to the CONNECT call.

Before:

% echo $'CONNECT nodns.boo HTTP/1.1\r\n' | nc localhost 8080
HTTP/1.1 502 Bad Gateway

After:

% echo $'CONNECT nodns.boo HTTP/1.1\r\n' | nc localhost 8080
HTTP/1.1 502 Bad Gateway
Content-Type: text/plain
Content-Length: 40

dial tcp: lookup nodns.boo: no such host

@nkns165
Copy link

nkns165 commented Oct 31, 2023

@elazarl It looks like a good change, why don't you merge it? I'd love to have this warning log.

@elazarl
Copy link
Owner

elazarl commented Oct 31, 2023

@nkns165 I'm worried it'd log too much for some people, maybe we need a better tunable logs.

@elazarl elazarl merged commit 3ec0782 into elazarl:master Oct 31, 2023
@nkns165
Copy link

nkns165 commented Oct 31, 2023

Thank you so much...!

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

Successfully merging this pull request may close these issues.

3 participants