ForzaElixir / rollbax

Exception tracking and logging from Elixir to Rollbar
https://hexdocs.pm/rollbax
ISC License
243 stars 52 forks source link

Allow to configure rollbax to contact api server via proxy #105

Closed GermanDZ closed 5 years ago

GermanDZ commented 5 years ago

Why?

Because some security hardened production environments require to contact the "outside" internet through a proxy and hackney (the erlang http client used by rollbax) doesn't read the usual env vars used to config a proxy.

How?

Adding an optional entry to Rollbar.config allowing the specification of an URL to be passed to hackney to force the use of a proxy.

Caveats

Kudos to @amuino for guiding me to this solution

GermanDZ commented 5 years ago

ping @whatyouhide

whatyouhide commented 5 years ago

Hey @GermanDZ, thanks for the contribution! I like it but all the infrastructure for testing it seems like overkill considering that we're basically testing a hackney feature. I would either:

Thoughts?

GermanDZ commented 5 years ago

I cannot use Mox to mock :hackney, being it an Erlang module without a behaviour (ArgumentError) module :hackney is not a behaviour, please pass a behaviour to :for)

My options right now:

1) Keep the Proxy server for test (current solution) 2) Add an indirection, extracting :hackney from the Rollbax.Client and using something like http_client/0, then for production http_client/0 will return the :hackney module, but during test we can mock it to return an alternative. This also could lead to allow to use other http client via configuration (but not seems too desirable) 3) Remove the test for being only a simple config (but I don't code without test)

whatyouhide commented 5 years ago

1 is what I would really prefer to avoid. 2 is what I meant when I said we could use Mox. I personally would go with 3, but either 2 or 3 are okay for me.

GermanDZ commented 5 years ago

Working on 2

GermanDZ commented 5 years ago

Finally I went for 3, no so much tests. Anyway, I've added a simple test just to ensure nobody removes the config without breaking the feature.

Now only check the request are made to the URL configured as proxy not the api_endpoint.

whatyouhide commented 5 years ago

Thanks @GermanDZ 💟