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

fix(xhr): allow setters for non-existing properties #477

Merged
merged 5 commits into from
Nov 8, 2023

Conversation

kettanaito
Copy link
Member

@kettanaito kettanaito commented Nov 7, 2023

Investigation

This issue is caused by the Axios XHR adapter in Happy DOM only. Axios does:

request.timeout = config.timeout

Where request is already a proxy established by Interceptors. It looks up the setProperty trap on the proxy and since timeout doesn't have a special case, it defaults to invoke().

Calling invoke() to set timeout on XMLHttpRequest proxy returns false because property timeout does not exist on target:

if (!(propertyName in target)) {
return null
}

The timeout property is not mandatory on XMLHttpRequest, so it's missing from the immediate XHR instance as well as its prototype. We return false from the setter and that causes the following error:

TypeError: 'set' on proxy: trap returned falsish for property 'timeout'

Solution

I think us forbidding setters on unknown properties is a mistake. To fix this, I'm adding a fallback to propertySource to be the immediate target itself is findPropertySource returned null.

@kettanaito kettanaito force-pushed the test/axios-xhr-adapter branch from 59e0e4e to 46a4cf2 Compare November 7, 2023 10:49
@kettanaito kettanaito changed the title test(xhr): add axios xhr adapter test fix(xhr): allow setters for non-existing properties Nov 8, 2023
const propertySource = findPropertySource(target, propertyName)
if (propertySource === null) return false

const propertySource = findPropertySource(target, propertyName) || target
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual fix: target is now considered a fallback for propertySource if setting properties that don't exist on target or its prototype.

@kettanaito kettanaito merged commit 4e73ed9 into main Nov 8, 2023
1 check passed
@kettanaito kettanaito deleted the test/axios-xhr-adapter branch November 8, 2023 09:50
@kettanaito
Copy link
Member Author

Released: v0.25.9 🎉

This has been released in v0.25.9!

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

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

Successfully merging this pull request may close these issues.

TypeError: 'set' on proxy: trap returned falsish for property 'timeout' (Axios, XHR, Happy DOM)
1 participant