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

revert: "fix: pass original fetch args along" #1371

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

pauldambra
Copy link
Member

Reverts #1351

we've seen multiple reports of fetch wrapping causing errors, it would be amazing if it wasn't this change causing it.

it's safe to revert if it's not this change and important to revert if it is...

Copy link

vercel bot commented Aug 20, 2024

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

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Aug 20, 2024 7:35am

@pauldambra pauldambra added the bump patch Bump patch version when this PR gets merged label Aug 20, 2024
@pauldambra pauldambra changed the title revert "fix: pass original fetch args along" revert: "fix: pass original fetch args along" Aug 20, 2024
Copy link

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

@benjackwhite benjackwhite merged commit a673570 into main Aug 20, 2024
13 of 14 checks passed
@benjackwhite benjackwhite deleted the revert-1351-fix/pass-original-along branch August 20, 2024 08:15
@ricardobeat
Copy link

The same error has been fixed in dd-trace a while ago by calling fetch with the original input Request object: https://github.com/DataDog/dd-trace-js/pull/4258/files

@pauldambra
Copy link
Member Author

@ricardobeat interestingly that's a fix in DD for a different issue. their thread mentions it is difficult to test for but I'll see if i can poke about and recreate for us since we might be open to the same error as well (although haven't had it reported)

@ricardobeat
Copy link

@pauldambra the core issue is the same, wrapping fetch incorrectly. I meant to point to a potentially better solution vs the one at 24979cb which has been reverted, preserving a Request object if received is more reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants