Whitelisted requests still raise errors if response code is not successful

[digitalcora]

I'm working on a project that uses subdomains extensively, so in our test environment the app host is lvh.me, an internet domain with wildcard DNS that resolves to 127.0.0.1. Following the advice of the README, which says:

By default, all requests to localhost or 127.0.0.1 will not be cached. If you're running your test server with a different hostname, you'll need to add that host to puffing-billy's whitelist.

...I have added /lvh\.me/ to Billy's whitelist. However, requests to the test server are still being logged as warnings if the response is a redirect or a 4xx, or blocked outright if non_successful_error_level is set to :error, which causes tests to fail. Shouldn't the whitelist cause the test server to be exempt from these checks? Or is the whitelist not supposed to be used for something like this?

(on a related note, I never saw any actual errors raised when setting non_successful_error_level to :error – requests were just getting silently blocked, and I had to dig through the logs to figure out what was going on. are there any known issues with this?)

[ronwsmith]

Your interpretation of the whitelist seems correct. Can you show me your full Billy config?

Not sure about the blocking issue. I know it was difficult to bubble up errors initially but haven't heard any issues before now.

[digitalcora]

This is the entire configuration:

Billy.configure do |config|
  config.whitelist += [/lvh\.me/]
  config.non_successful_error_level = :error
end

Using this, even when requests are sent to an lvh.me URL, if the response is a redirect or has a non-successful status, Billy logs an error and blocks the request (or if the error level is set to :warn instead, it logs a warning). If I enable caching I can see that lvh.me requests are correctly not being cached, so the whitelist is having some effect.

[ronwsmith]

Ah ha, I see why now. It's here: https://github.com/oesmith/puffing-billy/blob/master/lib/billy/handlers/proxy_handler.rb#L34-L40. It always logs what the status code is for non-successful cases.

This was added because people were having issues with redirects or 404s being cached silently. Perhaps we should change it to :info or allow the option to add custom codes to allowed_response_code?. Do you have a preference?

[digitalcora]

This was added because people were having issues with redirects or 404s being cached silently.

Since whitelisted requests are never cached, this shouldn't be an issue in that case. Shouldn't they also bypass everything else the proxy handler normally does? That was my expectation when I initially read the docs. (I might want to raise an error if I call an external service and it 404s, but if the local API on my test server issues a redirect or 404, that might be a necessary part of its operation that I want to be able to test without raising an error)

[ronwsmith]

Since puffing-billy's implementation is as a driver proxy, all URLs go through the proxy handler or else they won't be fetched at all. The only exception is localhost/127.0.0.1 because of an open issue with phantomjs.

We could add a whitelist check to allowed_response_code? to effectively bypass anything after the request (we already do something similar in cacheable?).

[digitalcora]

We could add a whitelist check to allowed_response_code? to effectively bypass anything after the request (we already do something similar in cacheable?).

That sounds like a good approach to me! Or perhaps the "bypass normal processing for whitelisted requests" could happen at a higher level, so that if any additional things (like the allowed-response-codes filter) are added later, whitelisting will also bypass those.

[ronwsmith]

@Grantovich is this issue still something you'd like to see resolved?

[digitalcora]

@ronwsmith Thanks for following up! I haven't worked on any projects that use puffing-billy for some years, so I'm not sure if this issue still exists in the exact form it's described here, and have no personal stake in it. If it does still exist, perhaps someone else can take ownership 🙂

[ronwsmith]

Closing, stale issue. I can reopen if someone else runs into the same issue.