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.

Sign up for GitHub

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

Jump to bottom

[pkg/driver] XHR: Maximum call stack size exceeded #5864

Closed
kirbycool opened this issue Dec 3, 2019 · 1 comment · Fixed by #5945 · May be fixed by rangle/test-automation#29
Closed

[pkg/driver] XHR: Maximum call stack size exceeded #5864

kirbycool opened this issue Dec 3, 2019 · 1 comment · Fixed by #5945 · May be fixed by rangle/test-automation#29

Comments

@kirbycool
Copy link
Contributor

Current behavior:

In the current xhr code, the getter for onreadystatechange wraps the original callback. However, it overwrites the memoized original callback with the wrapper, so every time the getter runs, an additional wrapper is added. If xhr.onreadystatechange is accessed enough times, you can end up with an extremely long call stack calling into the nested wrappers.
See https://github.com/cypress-io/cypress/blob/develop/packages/driver/src/cypress/server.coffee#L498

This is xhr specific and not related to #4373

Desired behavior:

xhr callback wrappers do not nest an cause ballooning call stacks.

Steps to reproduce: (app code and test code)

I ran into this issue when using pollyjs to record xhrs, which uses sinon/nise to intercept and record xhrs.
It emulates a chunked response by loading the body in chunks, calling onreadystatechange for each chunk.
https://github.com/sinonjs/nise/blob/1a3eedd09fca7c27ec2212c4a04e62a56a2b0d46/lib/fake-xhr/index.js#L675

Every time onreadystatechange is called, a new wrapper is added by cypress. For a sufficiently long body, it's called enough times to cause the stack error.

I can find more time later to build a reproducible example if you need it.

Versions

I think changing

              if _.isFunction(bak)
                fns[prop] = ->
                  bak.apply(xhr, arguments)

to

              if _.isFunction(bak)
                () -> bak.apply(xhr, arguments)

should be enough to solve the problem. Memoizing the user supplied callback is done in the setter already, so it doesn't need to be done here as well.
@kirbycool kirbycool changed the title XHR: Maximum call stack size exceeded [pkg/driver] XHR: Maximum call stack size exceeded Dec 12, 2019
kirbycool added a commit to kirbycool/cypress that referenced this issue Dec 12, 2019
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
094a664 
- Closes cypress-io#5864

Fix deep call stacks for XHRs

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter

N/A

N/A

- [ ] Have tests been added/updated?
- [ ] Has the original issue been tagged with a release in ZenHub? <!-- (internal team only)-->
- [ ] Has a PR for user-facing changes been opened in [`cypress-documentation`](https://github.com/cypress-io/cypress-documentation)? <!-- Link to PR here -->
- [ ] Have API changes been updated in the [`type definitions`](cli/types/index.d.ts)?
- [ ] Have new configuration options been added to the [`cypress.schema.json`](cli/schema/cypress.schema.json)?
kirbycool added a commit to kirbycool/cypress that referenced this issue Dec 12, 2019
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
636ee6a 
- Closes cypress-io#5864

Fix deep call stacks for XHRs

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter

N/A

N/A

- [ ] Have tests been added/updated?
- [ ] Has the original issue been tagged with a release in ZenHub? <!-- (internal team only)-->
- [ ] Has a PR for user-facing changes been opened in [`cypress-documentation`](https://github.com/cypress-io/cypress-documentation)? <!-- Link to PR here -->
- [ ] Have API changes been updated in the [`type definitions`](cli/types/index.d.ts)?
- [ ] Have new configuration options been added to the [`cypress.schema.json`](cli/schema/cypress.schema.json)?
kirbycool added a commit to kirbycool/cypress that referenced this issue Dec 12, 2019
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
c6aea8a 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
kirbycool added a commit to kirbycool/cypress that referenced this issue Dec 12, 2019
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
e4e8aea 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
@kirbycool kirbycool mentioned this issue Dec 12, 2019
Merged
5 tasks
kirbycool added a commit to kirbycool/cypress that referenced this issue Dec 16, 2019
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
684a4e7 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
kirbycool added a commit to kirbycool/cypress that referenced this issue Dec 16, 2019
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
51ddceb 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
kirbycool added a commit to kirbycool/cypress that referenced this issue Jan 7, 2020
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
1c617b1 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
kirbycool added a commit to kirbycool/cypress that referenced this issue Jan 7, 2020
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
8247f97 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
kirbycool added a commit to kirbycool/cypress that referenced this issue Jan 7, 2020
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
39b19fe 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
kirbycool added a commit to kirbycool/cypress that referenced this issue Feb 17, 2020
@kirbycool
[driver] Fix xhr ballooning callback stack depth.
276ccce 
- Closes cypress-io#5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter
flotwig added a commit that referenced this issue Feb 18, 2020
@kirbycool @flotwig
[driver] Fix xhr ballooning callback stack depth. (#5945)
b5c9572 
* [driver] Fix xhr ballooning callback stack depth.

- Closes #5864

Cypress wraps xhr callbacks with an override.  However, in the current code, every time a callback getter is called, we replace the original memoized callback with the wrapper. Every time the getter is called, another wrapper is added, which in extreme cases (e.g. iterating over chunks of a long request body and calling `onreadystatechange` for each chunk) can cause "Maximum call stack size exceeded" errors.

This change resolves the issue by not overwriting the memoized callback in the getter. The setter

* Update xhr_spec.coffee

Co-authored-by: Zach Bloomquist <github@chary.us>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 28, 2020

Released in 4.1.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.1.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Feb 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@kirbycool