jameslnewell / xhr-mock

Utility for mocking XMLHttpRequest.
196 stars 48 forks source link

Unclear URL matching rules with param encoding #63

Open dobriai opened 6 years ago

dobriai commented 6 years ago

I figured it out, after some debugging, but it should be either changed or documented better.

EXAMPLE: I have a myUrl with some search params like this: https://developer.api.acmecorp.com/content/v1/contentsSearch?filter[contents][libraryId]=urn:acme.content:library:a0d5b7b4-85cb-46f1-8df5-294f16993702&filter[contents][entityType]=content&filter[contents][searchSkipCache]=True&filter[contents][query]=title:Ellipse&page[size]=50&page[number]=1 (Pardon the extra length, it could probably be trimmed a bit for a demo - I know!)

Then I prepare to receive the call like so:

    mock.get(myUrl, (req, res) => {
...
    });

And expect a call xhr.open('get', myUrl) somewhere in the code to be handled by the mock. Instead I get an Error like this:

console.error node_modules\xhr-mock\lib\MockXMLHttpRequest.js:671 xhr-mock: No handler returned a response for the request.

GET https://developer.api.acmecorp.com/content/v1/contentsSearch?filter%5Bcontents%5D%5BlibraryId%5D=urn%3Aacme.content%3Alibrary%3Aa0d5b7b4-85cb-46f1-8df5-294f16993702&filter%5Bcontents%5D%5BentityType%5D=content&filter%5Bcontents%5D%5BsearchSkipCache%5D=True&filter%5Bcontents%5D%5Bquery%5D=title%3AEllipse&page%5Bsize%5D=50&page%5Bnumber%5D=1 HTTP/1.1 content-type: application/vnd.api+json accept: application/vnd.api+json

Of course, stuff has been encoded! But how exactly?! Something like encodeURI(myUrl) gets me oh-so-close, but no thanks! It leaves the ':'-s intact, unlike the path displayed in the error above.

After some digging, it seems that xhr-mock's internally compares to something like this, during the lookup phase:

const encodedUrl = url.format( {...(url.parse(myUrl, true)), search: undefined} );
mock.get(encodedUrl, (req, res) => {
...

At least this works in my case!

I am not sure if this is a bug or not, but as a minimum it should be documented, if not fixed. The ideal fix being "One can use the same URLs for xhr.open() and mock.xxx()."

Very nice package - many thanks for the effort!

jameslnewell commented 6 years ago

👍Thanks for the report.

"One can use the same URLs for xhr.open() and mock.xxx()."

Yeah that sounds ideal. Are you able to submit a PR for the fix?

Very nice package - many thanks for the effort!

Thanks!

dobriai commented 6 years ago

OK, I will try to think of a reasonable fix. The thing is I don't know that much about URL encoding and all of its facets, so I'll have to educate myself a bit more.

Do you have any opinions on what the fix would look like? At first glance I can imagine:

  1. An aggressive approach: Modify mock.get() et al to transform the URL appropriately. This would be swell, but might break something that I am not aware of

  2. A passive approach: Add a separate utility that encodes the URL, to be called by the user. This is quite safe, but requires the user to ... well, use that new API :-)

  3. Then there are probably other options - maybe add some optional arg somewhere, which tells whether the passed URL should be encoded or left "as is".

I will think about it ... when I have some time. But let me know if you have any preferences or thought on the subject!

jameslnewell commented 6 years ago

Yeah I'd prefer to match URLs the same as passed to xhr.open() without having to encode them for xhr-mock. I'd consider having to encode the URL for xhr-mock (and not xhr.open()) a bug... hopefully that's not affecting too many people as it hasn't been reported before.