Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@sentry/vue breaks Google Analytics CORS policy #5868

Closed
3 tasks done
9mm opened this issue Oct 3, 2022 · 17 comments
Closed
3 tasks done

@sentry/vue breaks Google Analytics CORS policy #5868

9mm opened this issue Oct 3, 2022 · 17 comments
Assignees

Comments

@9mm
Copy link

9mm commented Oct 3, 2022

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/vue

SDK Version

7.14.0

Framework Version

7.14.0

Link to Sentry event

No response

Steps to Reproduce

  1. Add Sentry to vue app main.js:
import * as Sentry from '@sentry/vue'; 
import { Integrations } from '@sentry/tracing'; 

if (process.env.NODE_ENV === 'production') {
  Sentry.init({
    vm,
    dsn: 'https://12345@12345.ingest.sentry.io/12345',
    integrations: [
      new Integrations.BrowserTracing({
        routingInstrumentation: Sentry.vueRouterInstrumentation(router),
        tracingOrigins: [
          'localhost',
          'example.com',
          'example2.com',
          /^\//,
        ],
      }),
    ],
    tracesSampleRate: 0.05,
  });
}
  1. Implement analytics.js form of Google Analytics code... This part isn't super important, all you have to know is that OUR code does not handle the ajax request (obviously, if you've ever used Google Analytics this makes sense). Instead, you interact with ga object provided by Google For example...
window.ga('create', '12345', 'auto');
window.ga('set', 'title', title);
window.ga('send', 'pageview', { page: document.location.pathname, title });
  1. After firing a pageview event, look in the browser and you will see ga trying to make a CORS request...

image

if you look at the exact reason it fails, its because Google responds with Access-Control-Allow-Origin: * but the request itself is using withCredentials: include...

Access to XMLHttpRequest at 'https://www.google-analytics.com/j/collect?v=1&_v=j97&a=xxxxxxxx&t=pageview&_s=1&dl=URLSTUFF' from origin 'https://www.example.com' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'. The credentials mode of requests initiated by the XMLHttpRequest is controlled by the withCredentials attribute.

I just thought what could "wrap" these requests which might be modifying them. I disabled sentry in main.js and the analytics requests now work perfectly

Expected Result

I honestly have no idea even WHAT is happening, or why Sentry would need to modify withCredentials on xmlhttprequests.... maybe that's not even happening, but but is triggering this error indirectly

The expected result should be google analytics should work normally, like when sentry is not loaded

Actual Result

The actual way I found this is by looking at the error pasted above in chrome dev tools console. You will see the 'initiator' file of the error. Instead of analytics.js being the initiator (which youd expect), I noticed it was a babel chunk js file..... When I clicked either of these 2 exceptions below it brings me to Sentry code, which is telling me that whatever code Sentry is wrapping is breaking the way Google needs it to work

image

@Lms24
Copy link
Member

Lms24 commented Oct 3, 2022

Hi @9mm thanks for writing in!

Interesting problem. Are you able to compare the outgoing GA requests when using the Sentry SDK vs. when not using it?

AFAICT, the only thing we modify on outgoing requests is that we add headers, namely the sentry-trace and baggage headers (both are necessary for distributed tracing). This can cause CORS issues but only if the headers are actually attached to the requests. Which in turn only happens, if the URL of the outgoing requests is matched by the strings or regexes specified in the tracingOrigins property of our BrowserTracing integration. Based on your provided list of tracingOrigins I can't see how this would work but maybe you can double check, if these headers are ending up in the outgoing GA requests.

@9mm
Copy link
Author

9mm commented Oct 3, 2022

Can you send me your email and I can email the site to you? Maybe you can also look internally at the Trace ID's below or something

It's hard to tell regarding your question because when the request fails, it shows as 2 requests, OPTIONS / POST+Preflight. When the request works it shows as POST. From what I see they're mostly the same.

Broken

POST

baggage: sentry-transaction=category_group,sentry-public_key=8f2b9a97b1784f53aab9767160ef24d2,sentry-trace_id=41062cfbecb9455aa84f4b4324fcdfe1,sentry-sample_rate=0.05
Content-Type: text/plain
Referer: https://www.example.com/
sec-ch-ua: "Chromium";v="106", "Google Chrome";v="106", "Not;A=Brand";v="99"
sec-ch-ua-mobile: ?0
sec-ch-ua-platform: "macOS"
sentry-trace: 41062cfbecb9455aa84f4b4324fcdfe1-a9e3d75c606eb4d0-0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/106.0.0.0 Safari/537.36

OPTIONS

:authority: www.google-analytics.com
:method: OPTIONS
:path: /j/collect?v=URL
:scheme: https
accept: */*
accept-encoding: gzip, deflate, br
accept-language: en-US,en;q=0.9
access-control-request-headers: baggage,sentry-trace
access-control-request-method: POST
origin: https://www.example.com
referer: https://www.example.com/
sec-fetch-dest: empty
sec-fetch-mode: cors
sec-fetch-site: cross-site
user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/106.0.0.0 Safari/537.36

WORKING

POST

:authority: www.google-analytics.com
:method: POST
:path: /j/collect?v=URL
:scheme: https
accept: */*
accept-encoding: gzip, deflate, br
accept-language: en-US,en;q=0.9
content-length: 0
content-type: text/plain
origin: https://www.example.com
referer: https://www.example.com/
sec-ch-ua: "Chromium";v="106", "Google Chrome";v="106", "Not;A=Brand";v="99"
sec-ch-ua-mobile: ?0
sec-ch-ua-platform: "macOS"
sec-fetch-dest: empty
sec-fetch-mode: cors
sec-fetch-site: cross-site
user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/106.0.0.0 Safari/537.36

@9mm
Copy link
Author

9mm commented Oct 3, 2022

I don't know if it helps, here are the 2 functions that sit above analytics.js in dev tools when you hover over "initiator" of the failed request, in order as they appear (the top being the most recent function in the exception chain)

I know the code is minified but I'm sure you can figure it out from keywords in it, etc

Req 1

return function(...e) {
                var n = this
                  , r = ["onload", "onerror", "onprogress", "onreadystatechange"];
                return r.forEach((t=>{
                    t in n && "function" === typeof n[t] && (0,
                    g.hl)(n, t, (function(e) {
                        var n = {
                            mechanism: {
                                data: {
                                    function: t,
                                    handler: (0,
                                    O.$P)(e)
                                },
                                handled: !0,
                                type: "instrument"
                            }
                        }
                          , r = (0,
                        g.HK)(e);
                        return r && (n.mechanism.data.handler = (0,
                        O.$P)(r)),
                        de(e, n)
                    }
                    ))
                }
                )),
                t.apply(this, e) <===============================
            }

Req 2

s.hl)(t, "send", (function(t) {
                    return function(...e) {
                        return this.__sentry_xhr__ && void 0 !== e[0] && (this.__sentry_xhr__.body = e[0]),
                        h("xhr", {
                            args: e,
                            startTimestamp: Date.now(),
                            xhr: this
                        }),
                        t.apply(this, e) <==================
                    }
                }
                ))

@Lms24
Copy link
Member

Lms24 commented Oct 3, 2022

So, I think it kinda makes sense that you have two functions above because regardless of if we're modifying an outgoing request, we're instrumenting it (e.g. for breadcrumbs) by default. Just to have it written somewhere, these are the two functions:

req1:
https://github.com/getsentry/sentry-javascript/blob/9ac839076fa1a41d979ae7cf49a420e6abe1382e/packages/browser/src/integrations/trycatch.ts#L147-L183

req2:
https://github.com/getsentry/sentry-javascript/blob/9ac839076fa1a41d979ae7cf49a420e6abe1382e/packages/utils/src/instrument.ts#L273-L287


What I find interesting is that in your first request (labeled with Broken->POST), there are the two headers I was talking about earlier (baggage and sentry-trace). I'm wondering how they made their way into this request. Can you confirm that this is in fact a request made from GA to GA servers? And can you also double check if you have some regex in your tracingOrigins that would match the URL of the outgoing request?

It's important to note that request URLs only have to partially match. For example:
tracingOrigins: ["sentry.io", /localhost/] would also match URLs like notsentry.io/api or thisisnotlocalhost.com/api/..., meaning that we'd add tracing headers to these requests.

@Lms24
Copy link
Member

Lms24 commented Oct 3, 2022

Can you send me your email and I can email the site to you?

Sure, can you see my public email address on my profile?

@9mm
Copy link
Author

9mm commented Oct 3, 2022

Yes.. that's actually a great point about the headers being sent on that request. I'm certain its to Google analytics:

image

When clicking the request:

image

Sorry for the cropping (Ill email you the domain)

If I scroll down a bit i'm definitely seeing the baggage header on the same request

As far as you mentioning "From" GA, I'm not sure what you mean by that (it's not a request originating from googles servers, its originating from our domain, but its being generated by the ga object, rather than some Ajax request we make via axios or fetch ourself)

Here is the exact tracingOrigins... Ive only modified the 3 domains we own to example (none of the domains i changed are external domains like google analytics, just our product domains).

Maybe the problem is the last line? ... a regex matching /? I forgot why that's there, I feel like I pasted it from your example code?

        tracingOrigins: [
          'localhost',
          'example.com',
          'example2.com',
          'example3.com',
          /^\//,
        ],

@Lms24
Copy link
Member

Lms24 commented Oct 4, 2022

Hi @9mm, just to clarify, what I meant with "from GA" is that the Analytics script you're downloading is sending these XHR requests that cause the problem.

So I think for now we should continue investigating why the headers end up in the request. The /^\// regex shouldn't be responsible as the ^ should only match URLs starting with /, meaning that all outgoing requests to the website's domain should me traced (i.e. the headers should be attached).
My advice would be to find out which entry in the array is causing headers to be attached. Start with an empty tracingOrigins: [] array and check if the headers are still attached. In that case, something might be broken on our end. Then re-add your entries one by one and check back.

I took a look at the entries you sent me and I can't find anything that is directly responsible for it. Maybe (just guessing) GA puts your domain name or something that matches the entries in tracingOrigins as a query param into the URL?

Also, can you set the debug: true flag in your Sentry.init properties? Perhaps there is an error or warning that might help us find out what's going on.

@andrisp
Copy link

andrisp commented Oct 4, 2022

I'm experiencing the same issue, and I'm also confused about the last entry in the tracingOrigins example: /^\//. To my understanding this should match any string which starts with an /, but not sure what this should match if the matching is supposed to be done against full url (including domain).

@Lms24
Copy link
Member

Lms24 commented Oct 4, 2022

I just checked and for XHR requests (XMLHttpRequest), the SDK uses the URL to check against tracingOrigins that is passed to the XMLHttpRequest.open method. Example:

const xhr = new XMLHttpRequest();
xhr.open("GET", "/api/user/5"); // the /^\// regex will match
xhr.open("GET", "https://someDomain.com/api/user/5"); // the /^\// regex will not match (but maybe another entry)

Here's the relevant code:
https://github.com/getsentry/sentry-javascript/blob/9ac839076fa1a41d979ae7cf49a420e6abe1382e/packages/utils/src/instrument.ts#L215-L231

@Lms24
Copy link
Member

Lms24 commented Oct 4, 2022

I'm experiencing the same issue

@andrisp Are you also using the Vue SDK? How does your BrowserTracing setup look like (particularly tracingOrigins)?

@andrisp
Copy link

andrisp commented Oct 4, 2022

@Lms24 yes, Vue 2.6.10 and Sentry 7.1.1.

      integrations: [
        new Integrations.BrowserTracing({
          routingInstrumentation: SentryVue.vueRouterInstrumentation(router),
          tracingOrigins: ["localhost", "mydomain.app", "mydomain", /^\//],
        }),
      ],

I'm using "mydomain" for local dev/tests.

By looking at the google analytics URLs which get blocked with the CORS error, they indeed have "mydomain" included somewhere in the query string, and there is the "bagage" header in the request.

I'll experiment with better regex strings to match exact domains not parts of the full string. I think that the Sentry documentation could be improved about this (about /^\// - how does this even work if match is done against full string URL including domain and I suppose schema too)

@Lms24
Copy link
Member

Lms24 commented Oct 4, 2022

By looking at the google analytics URLs which get blocked with the CORS error, they indeed have "mydomain" included somewhere in the query string, and there is the "bagage" header in the request.

That's probably what's causing the CORS error.

I think that the Sentry documentation could be improved about this (about /^// - how does this even work if match is done against full string URL including domain and I suppose schema too)

Agreed. I'll open up an issue in our docs repo to track this. Fwiw, we're going to replace tracingOrigins with tracePropagationTargets (already introduced it in our Node SDK) in the near future so we'll need to overhaul documentation anyway. However, I'll add this point anyway as it's relevant for both options.

@Lms24
Copy link
Member

Lms24 commented Oct 4, 2022

By looking at the google analytics URLs which get blocked with the CORS error, they indeed have "mydomain" included somewhere in the query string, and there is the "bagage" header in the request.

To add to this, I'm currently almost certain that this is what's causing the problem. I just reproduced this behaviour by adding Sentry (Browser SDK) and analytics.js to one of my test apps and I got the same errors. Moreover, I checked that my statement above about the GA URL containing the domain name is correct and I can indeed find my domain in these URLs. That's why the entries in tracingOrigins match, which causes the tracing headers to be attached to the GA requests.

Solution

This means, if you use GA, you have to make more robust tracingOrigins than just the domain. Either create a regex that checks that the domain is mentioned at the beginning (like /^https:\/\/yourDomain.com/) or try to narrow down the URLs/routes you actually want to track within your domain (a string should be enough). Something like 'yourDomain.com/api/v1/'.

This is documented in our tracingOrigins documentation

@9mm
Copy link
Author

9mm commented Oct 4, 2022

Ahhh!! Great thinking with realizing its also in the query string. Thank you!!! 🎉

I will do that now but that totally makes sense thats the problem so I'll close this pre-emptively

@9mm 9mm closed this as completed Oct 4, 2022
Lms24 added a commit to getsentry/sentry-docs that referenced this issue Oct 6, 2022
Clarify the meaning of the JS SDK's default values for the `tracingOrigins` option of the `BrowserTracing` integration. As pointed out in getsentry/sentry-javascript#5868 ([comment](getsentry/sentry-javascript#5868 (comment))), it is not obvious on first glance, to which requests tracing headers are attached if the default values are used. This PR therefore adds an explanation and slightly changes the ordering of the `tracingOrigins` description. 

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
lforst pushed a commit to getsentry/sentry-docs that referenced this issue Oct 10, 2022
Clarify the meaning of the JS SDK's default values for the `tracingOrigins` option of the `BrowserTracing` integration. As pointed out in getsentry/sentry-javascript#5868 ([comment](getsentry/sentry-javascript#5868 (comment))), it is not obvious on first glance, to which requests tracing headers are attached if the default values are used. This PR therefore adds an explanation and slightly changes the ordering of the `tracingOrigins` description. 

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
@sassomedia
Copy link

@Lms24, FYI, tracePropagationTargets fails the GA call while tracingOrigins works with the same options provided. I just spent quite a bit of time trying to figure out what was wrong with the options when it was the actual property that was causing the issue. I was trying to be proactive and use the newer property but it doesn't seem to work just yet.

Btw, not related to Vue; I'm using sentry.javascript.react@v7.12.1.

@Lms24
Copy link
Member

Lms24 commented Oct 25, 2022

Hi @sassomedia
tracePropagationTargets has only been introduced to the Node-based SDKs (in version 7.9.0) and not yet in our browser SDKs. This shouldn't yet be documented in any browser-based SDK (such as the React or Vue SDKs). I'm realizing that my comment above, didn't make this entirely clear, sorry about that.

It's a little hidden but #5285 tracks this future change along with other changes we want to make to control span and trace propagation on a more fine-grained level.

If (for some reason) tracePropagationTargets shows up as a valid option in your type definition or docs, please feel free to open up an issue about it.

@sassomedia
Copy link

Ah ok, that makes sense then. I'm actually troubleshooting another issue so I've been scouring docs as well as GitHub issues and came across tracePropagationTargets in various places. That's why I thought this was already a thing. My bad. Thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants