fix: pass original fetch args along

[pauldambra]

When wrapping fetch we take a url, init pair and create a request object

When we pass on and call the original fetch method we pass the request object. this is technically equivalent to the original but not literally equivalent which can cause problems if there are further wrappers on fetch

see: #1326

We actively don't want to alter the fetch details so it should be 100% safe to use the original args when calling the original fetch 🤷

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
posthog-js	✅ Ready (Inspect)	Visit Preview	Aug 19, 2024 0:19am

[github-actions]

Size Change: +6 B (0%)

Total Size: 1.17 MB

Filename	Size	Change
dist/array.full.js	333 kB	+2 B (0%)
dist/recorder-v2.js	110 kB	+2 B (0%)
dist/recorder.js	110 kB	+2 B (0%)

ℹ️ View Unchanged

Filename	Size
dist/array.js	154 kB
dist/exception-autocapture.js	10.4 kB
dist/main.js	155 kB
dist/module.js	154 kB
dist/surveys-preview.js	59.8 kB
dist/surveys.js	66 kB
dist/tracing-headers.js	8.26 kB
dist/web-vitals.js	5.79 kB

_{compressed-size-action}

[daibhin]

Change makes sense. I think we should be returning exactly the arguments we were given. Probably an oversight earlier on to have returned the constructed Request so worth fixing forward.

There's a risk we break this the other way e.g. customers have come to rely on the output of our fetch returning a Request object. We won't really know that without shipping.

My suggestion would be to get this out there as a minor bump. At least then people are more likely to be able to track the version bump if it does cause an issue for them. Secondly, there's always the escape hatch of staying on the previous version until they've had a chance to fix their reliant code (which should be trivial either way).

The only other alternative I can think of is passing res = await originalFetch(req, init) back but that feels less good. We'd be preserving current functionality at the expense of correctness. Given the points above it's probably just worth fixing everything now and dealing with any consequential issues that arise.

[weilandia]

Our app went down tonight because of an issue with Posthog overwriting fetch in this file. Not sure that this exact change is the culprit, but the changes made to window.fetch in recorder.ts made RTKQuery unusable and left us with a very confusing FE bug that came down to the Posthog assets loading and breaking fetch. We solved it in the interim by using cross-fetch as the fetch function in RTK.

https://us-assets.i.posthog.com/src/entrypoints/recorder.ts

[pauldambra]

oh yuck! so sorry @weilandia! will revert this change I've seen a few reports of similar and I'd be amazed if it was any of the other changes yesterday

[weilandia]

Thanks!