search

bbyars / mountebank

Over the wire test doubles
http://www.mbtest.org
MIT License
2k stars 262 forks source link

Check that http URLs don't have a path in proxy.to #727

Closed groves closed 1 year ago

groves commented 1 year ago

I spent longer than I'd like to admit debugging why my http proxy with a path was getting 404s. I finally saw in the docs that proxy.to is supposed to be a URL without a path. For future people like me who don't read the docs scrupulously enough, I wanted to add a warning or a failure on creating a proxy with a path.

This is my first stab at that. I'm guessing if we wanted to add something like this, we'd want to make it a warning in one release and then possibly make it an error in a followup release. Rather than guess at what upgrade experience we want, I decided to open a PR to discuss it.

I ran the tests before opening the PR and saw "http proxy stubs: should gracefully deal with bad urls" fail. Based on that, I'm guessing we don't want to disallow or warn on URLs with paths.

I still opened this as that surprises me a bit. As a user of the library, I'd much rather be told that my configuration is invalid when I create it than when I try to use it. Your thoughts on how to proceed if at all?

bbyars commented 1 year ago

I'm very sorry for how long it's taken to get back to you. Yes I agree with your approach, giving the error on creation instead of use makes much more sense to me. I just removed the failing tests you mentioned and had to slightly tweak the condition you used (node lists the protocol as "http:" with the colon at the end). Thanks so much.