warn if usePost is undefined (close #613) - #615
warn if usePost is undefined (close #613)#615Chris Sherlock (chrissherlock) wants to merge 2 commits into
Conversation
| { | ||
| Console.warn('usePost undefined'); | ||
| path = '/i'; | ||
| } |
There was a problem hiding this comment.
Couldn't we have
usePost = typeof usePost != 'undefined' && usePost && window.XMLHttpRequest && ('withCredentials' in new XMLHttpRequest());What do you think Anton Parkhomenko (@chuwy) ?
There was a problem hiding this comment.
I would like to see a warning. One of the orgs who uses this has managed to encode an absolutely massive object and because this variable is undefined they are using GET requests... and thus is getting 414 errors because the URI is enormous... if they picked up on the fact that they hadn't set usePost, then it would have alerted them to the issue more quickly.
There was a problem hiding this comment.
But if I understand it right - if website-owner just wants to use GET-requests with reasonable payload - we'll force all of his users see "usePost undefined" message in console, which doesn't look right for me. So, this change is not about undefined-check it's more about GET-check. But correct me if I'm wrong.
I'm quite against all log messages from tracker - in 99,9% of cases they hit console-aware users and not website-owners. But I'm quite keen to introduce something like debug-mode for tracker, where all such cases can be safely printed in development-environment and deleted in production.
If we want to avoid 414-like issues - all we can do is to be more explicit in tracker documentation.
There was a problem hiding this comment.
Another way to fix it would be to use POST by default and allow to use GET by settings useGet = true, but this is quite big change for me, taking in account that CloudFront collector doesn't support POST.
Snowplow CLA bot (snowplowcla)
added
the
cla:yes
|
Chris Sherlock (@chrissherlock) has signed the Individual Contributor License Agreement. Thanks so much |
|
Agree with Anton Parkhomenko (@chuwy) - we won't add warnings into a production tracker. Debug mode is the way to go. |
|
This has been superseeded by |
7 participants
Sorry about before, this patch should hopefully be better.