How to stop appending 'null' to the url?

[phillip-haydon]

Any request I make, i.e:

quest.get('https://mysite.com/stuff')
         .then(....);

Ends up calling:

https://mysite.com/stuff?null

How can I stop it from adding ?null to the url?

I forgot to mention, I have set cache: true and using latest version, it still appends the null and prevents caching.

[pyrsmk]

Ahah! Good one!

It comes from your data parameter that you've set to null, and the addition of a cache: true. I (probably) fixed it with the last push. Can you verify and tell me if all is working as well?

[phillip-haydon]

@pyrsmk I will test in a couple of hours when I'm home from work.

So what I noticed was:

quest.get('https://mysite.com/stuff')

This causes null to be in the URL, even if you have no options. So I figured, because caching is off by default, this should be appending a timestamp. But it actually appends null.

So I thought I'll set it to be cachable, which then forced me to put null which didn't fix the problem.

I assume that's the issue you've just fixed?

I think the former may be an issue too. Also if you do:

quest.get('https://mysite.com/stuff', {})

That causes the URL to generate with ?{} as the query string.

[pyrsmk]

Yes, the last two bugs you're mentioning should be fixed by now. But the first surely comes from another place. Weird... What browser are you using?

[phillip-haydon]

Using Chrome latest version. On Windows 10. I'll verify all when I get home and let you know.

[pyrsmk]

As reading the code, I think that the first bug should be fixed too with the last commit.

In fact, qwest does not have cache busting anymore with a __t= parameter (since when? I can't find the related commit ^^). It now sets Cache-Control to no-cache when cache is set to false (by default).

[phillip-haydon]

Hmm the reason I picked this up was last night I saw cache control no cache being sent to aws gateway api. Even after setting cache true.

I just looked at the commit and it looks like it does solve both the first issues.

[phillip-haydon]

It looks like the code sets the cache control header correctly when cache is set to false. Maybe I imagined it. I shall verify!

[pyrsmk]

Ahah ok! Everything seems to be fine by now! I'll wait for your confirmation. Anyway, thanks a lot for your contribution ;)

[phillip-haydon]

Ah thank you for creating an awesome lib.

The bug is still occurring, I'm trying to figure out why. Had to hack up the quest.js file to get it working since I'm not using browserfy.

[phillip-haydon]

FOUND IT!

https://github.com/pyrsmk/qwest/blob/master/src/qwest.js#L310

This line is turning it to a string "null" which means the if statement always evaluates as true.

[phillip-haydon]

Updated it to data = data ? JSON.stringify(data) : data; locally and it's working correctly now.

[pyrsmk]

Thanks a lot! I'm fixing this

[pyrsmk]

Ah... I wasn't aware of that automatic closing according to a commit label...

Can you test if you have time? ^^

[phillip-haydon]

Saw the commit in email.

Just finished testing, updated via npm, using the .min, worked PERFECTLY! :D

Now, I just need to figure out why saying cache: true still add's Cache-Control:no-cache

[pyrsmk]

Wut Oo

What are your other options for the request?

[phillip-haydon]

        qwest.get('https://***/help/' + opts.for, null, {
            cache: true
        }).then((xhr, response) => {
            div.innerText = response.text;
        });

That's it.

[pyrsmk]

I can't understand why qwest is still setting this header, it should not be the case if cache is set to true... There is just two lines for that in the source. Can you put a console.log at line 339 to verify if qwest is setting that header?

[phillip-haydon]

It doesn't hit that line at all.

It only sets it in Chrome, it doesn't hit that line at all. It's like the browser is adding it, but the W3 spec says:

If the user agent implements a HTTP cache it should respect Cache-Control headers in author request headers (e.g. Cache-Control: no-cache bypasses the cache). It must not send Cache-Control or Pragma request headers automatically unless the end user explicitly requests such behavior (e.g. by reloading the page).

So confusing, I don't believe this issue is with qwest. I'll try and figure it out tho :(

[pyrsmk]

It's only happening in Chrome? What the hell...
I remember that I had many problems when I wrote qwest's cache tests. I think I had one issue simular to the one you encounter, but I can't figure out how I fixed it... ^^;

Is your request using CORS?

[phillip-haydon]

Yes, it's using CORS, but the cache time is 1 hour so I wonder if it has cached the no-cache headers from before. I'll have to wait it out and see.

[pyrsmk]

Only the request body is cached. I think this bug could be related to #90

edit : my last comment could be a temporary fix to your problem (probably)

[phillip-haydon]

When CORS you have a preflight OPTIONS request, that can be cached to avoid additional requests having to issue preflight requests also. My options is set to cache for 1 hour. accept/cache-control are both headers which are added to the allowed headers. I think the options is cached for an hour and the browser uses that when setting up the XMLHttpRequest object.

Access-Control-Allow-Headers 'Content-Type,X-Amz-Date,Authorization,X-Api-Key,accept,cache-control'

[pyrsmk]

Ah, I didn't know that. CORS requests are a bit complicated.

[phillip-haydon]

@pyrsmk can confirm, it was cached on the options. Today I'm not getting any cache-control headers. Everything is perfect now! Thanks for fixing that issue so quickly for me.

[pyrsmk]

Great!