segmentio / analytics-next

Segment Analytics.js 2.0
https://segment.com/docs/connections/sources/catalog/libraries/website/javascript
MIT License
408 stars 136 forks source link

WebdriverIO tests #739

Closed ksubramanian2 closed 1 year ago

ksubramanian2 commented 1 year ago

We have some webdriverio tests to verify the Segment tracking calls in our pages. It was working Ok with the earlier version - analytics/core. We have upgraded to analytics/next and the fetch interceptor that comes with wdi is not seeing the Segment calls any more. It is seeing all the other network calls. Is there anything different about the way Fetch is handled in analytics/next?

silesky commented 1 year ago

@ksubramanian2. We use "unfetch", which is a fetch polyfill -- but we haven't heard of anyone having trouble with interception. I'm not sure what would be wrong here. I would double check that the HTTP Request matcher is still correct, and that events actually do get sent, even if they aren't being intercepted.

Maybe you can you post your intercept code?

ksubramanian2 commented 1 year ago

@silesky. Thanks for the quick response. I have verified that the events do get sent. Just our test is not caching them. Anything different in the new requests? The interceptor code from wdio is here: https://github.com/webdriverio-community/wdio-intercept-service/blob/2d09c6fef866f83cc2dade0f3c77426877262d61/lib/interceptor.js#L57

Wdio includes this service: https://webdriver.io/docs/wdio-intercept-service/ I use this service so my intercept code is just one line: browser.setupInterceptor(); Then later I would do browser.getrequests(). This is when I used to see the Segment call.

ksubramanian2 commented 1 year ago

@silesky This could be a problem here. https://github.com/segmentio/analytics-next/blob/ec568e5c1aa18970aef9eb482872b71328e758c9/packages/browser/src/plugins/segmentio/fetch-dispatcher.ts#L3 It has the code below. 'fetch' variable is getting set at module initialization and even though the correct window.fetch (with interceptor) is passed to the analytics.page call later, dispatch is not using the current window.fetch, but uses the stored value of 'fetch'. One fix would be to make the assignment to 'fetch' inside the dispatch function. Then it would use the latest value.

import unfetch from 'unfetch'

let fetch = unfetch if (typeof window !== 'undefined') { fetch = window.fetch || unfetch }

export type Dispatcher = (url: string, body: object) => Promise

export default function (): { dispatch: Dispatcher } { function dispatch(url: string, body: object): Promise { return fetch(url, { headers: { 'Content-Type': 'text/plain' }, method: 'post', body: JSON.stringify(body), }) }

return { dispatch, } }

silesky commented 1 year ago

@silesky This could be a problem here.

https://github.com/segmentio/analytics-next/blob/ec568e5c1aa18970aef9eb482872b71328e758c9/packages/browser/src/plugins/segmentio/fetch-dispatcher.ts#L3

It has the code below. 'fetch' variable is getting set at module initialization and even though the correct window.fetch (with interceptor) is passed to the analytics.page call later, dispatch is not using the current window.fetch, but uses the stored value of 'fetch'. One fix would be to make the assignment to 'fetch' inside the dispatch function. Then it would use the latest value. import unfetch from 'unfetch'

let fetch = unfetch if (typeof window !== 'undefined') { fetch = window.fetch || unfetch }

export type Dispatcher = (url: string, body: object) => Promise

export default function (): { dispatch: Dispatcher } { function dispatch(url: string, body: object): Promise { return fetch(url, { headers: { 'Content-Type': 'text/plain' }, method: 'post', body: JSON.stringify(body), }) }

return { dispatch, } }

So your theory is that the interceptor doesn't stub fetch until too late into the initialization -- seems plausible. I'd expect there to be an existing issue in the wdio repo, since I can imagine people run into this in plenty of different (non-analytics) testing scenarios.

I don't know of any reason off-hand why we couldn't tweak this code to get around the shortcomings of wdio, but would you be able to test this / create a minimal example repo that confirms that your suggestion indeed fixes the issue?

ksubramanian2 commented 1 year ago

@silesky I was able to test this tweak. With the fix, it is able to report on the Segment network calls and we are able to verify all the properties. Sample test output: [0-0] Number of network requests at firstpagecall = 9 [0-0] https://segment.intuitcdn.net/v1/projects/NSfYSMcSLpa4rH2wm2VxshxHRk3hJQE8/settings [0-0] https://eventbus-e2e.intuit.com/v2/segment/sbseg-qbo-clickstream/p

and we are able to verify that all the properties are getting populated correctly in our application, like the extract below. [0-0] { [0-0] "timestamp": "2022-12-29T08:34:40.840Z", [0-0] "integrations": { [0-0] "Adobe Analytics": { [0-0] "marketingCloudVisitorId": "16444118304720754581986161952403509320", [0-0] "imsregion": 9 [0-0] } [0-0] }, [0-0] "anonymousId": "08c030f2-545c-4cff-86a3-d2ea3432a4e8", [0-0] "type": "page", [0-0] "properties": { [0-0] "path": "/trackstar-test.html",

I have put the fix in the form of a PR - https://github.com/segmentio/analytics-next/pull/740 Trust this is Ok. Thanks.

ksubramanian2 commented 1 year ago

@silesky if there is a test mode and if you want me to enable this fix only in the test mode, please let me know how I can pass the 'testing' flag.