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

Log 407 responses from proxy #85

Merged
merged 2 commits into from
Apr 25, 2022
Merged

Log 407 responses from proxy #85

merged 2 commits into from
Apr 25, 2022

Conversation

samuong
Copy link
Owner

@samuong samuong commented Apr 20, 2022

When a proxy server responses with 407 Proxy Authentication Required, Alpaca silently retries the request with authentication. This PR adds logging so that we know when Alpaca has gone down the code path to authenticate to the proxy. This makes it easier to see what's going on in cases such as issue #44.

@samhaque
Copy link

I spoke with a few users who have requested to set verbosity on the log messages via some environment variable, as they said it's too noisy.

Also I believe when installed as a brew service the logs are sent to dev null, we can also look at piping the logs to a rotating log file.

@samuong
Copy link
Owner Author

samuong commented Apr 21, 2022

@samhaque is it too noisy because of this change specifically, or is that more of a general comment?

I'm wondering if it would help to try to rework this PR so that it (usually, unless there's an error somewhere else) prints a single line per request (e.g. GET http://example.com via "DIRECT", retried with auth) rather than two separate lines.

@samhaque
Copy link

So users said noisy in general but that's just how a proxy works, I guess a switch to turn the logging off is what the users are looking for, as there is a performance cost to consider when logging every successful proxy request.

@samuong
Copy link
Owner Author

samuong commented Apr 25, 2022

Ok, I'll land this as is then, and we can think about how to make it less verbose in a separate issue/pull request. Obviously it's easy enough to just redirect all output to /dev/null, but it would be good to get a better sense of what is still useful and what is noise.

@samuong samuong merged commit ce0461e into master Apr 25, 2022
@samuong samuong deleted the log-407s branch April 25, 2022 08:56
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.

None yet

3 participants