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

When get_pac returns None, there is no way to tell if there is no PAC, or if error occurred #76

Open
KarelChanivecky opened this issue Aug 2, 2023 · 3 comments

Comments

@KarelChanivecky
Copy link
Contributor

In download_pac, when an error occurs during an HTTP request, the error is ignored, and the next URL is attempted. At the end, if all URLs failed with error, the function returns None. It can be implied from this function that an error has occurred. If the list of URLs contained at least one URL, it should be expected that a response is available. However, it cannot be implied from get_pac, as perhaps PAC is not configured at all. Hence, either:

  • get_pac raises an error if PAC is not configured (This way the user can imply whether to expect a PAC)
  • get_pac evaluates if a PAC script was expected and raises an error otherwise
  • download_pac collects all the errors that occured, and chooses one to raise
  • download_pac collects all the errors and throws a new error type that contains the list of errors that occurred. (Leave it up to the user to figure out what error he cares about)
@KarelChanivecky
Copy link
Contributor Author

Today, I found there is a registry value from which one can infer what are the available proxy modes. Given this value, it could be evaluated whether the WPAD or the PAC URL should be evaluated at all. In case that they did, then one should expect one, hence failing of those request would deserve to be an exception:

https://stackoverflow.com/questions/1674119/what-key-in-windows-registry-disables-ie-connection-parameter-automatically-det

I implemented the suggestion above in our code base. I could contribute this here if the maintainers agree.

@carsonyl
Copy link
Owner

carsonyl commented Aug 5, 2023

That stackoverflow post is a very good find. But I am not sure about the idea of get_pac() being changed to raise errors by default, due to compatibility. The original idea was that missing PAC should not raise an error because the operating system doesn't do it either.

@KarelChanivecky
Copy link
Contributor Author

KarelChanivecky commented Aug 7, 2023

How about adding an API function to help the user evaluate if PAC/wpad is configured to be used?

This could be used to determine whether a PAC script is to be expected as a return from get_pac

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

No branches or pull requests

2 participants