Cannot redefine property: onreadystatechange

[frattaro]

• Operating System: Ubuntu
• Cypress Version: 0.19.4
• Browser Version: Electron 53

Is this a Feature or Bug?

Bug

Current behavior:

Running test causes error, test fails. Error:

Uncaught TypeError: Cannot redefine property: onreadystatechange 
    at .../__cypress/static/js/cypress.js:5452:27
    at Array.forEach
    at XMLHttpRequest.XHR.open (...cypress.js:5446:17)
    at XMLHttpRequest.XHR.open (...cypress.js:5469:23)

Desired behavior:

Test does not fail, test actually succeeded.

How to reproduce:

Run test

Test code:

describe('Support', function () {
  it('Searches', function () {
    cy.visit('https://www.mysite.com')
    cy.get('.global-nav-loginview').contains('Login').click()
    cy.wait(2000).get('label[for="UserName"]').click()
    cy.get('#UserName').type('username', { force: true })
    cy.get('label[for="Password"]').click()
    cy.get('#Password').type('********', { force: true })
    cy.get('.log-on-submit').click()
    cy.wait(2000).get('#steps a').contains('Support').click()
  })
})

Additional Info (images, stack traces, etc)

Unfortunately, logged in area so I can't send an exact reproduction. I hope the stack trace helps.

[jennifer-shehane]

Unfortunately, this is going to be quite difficult for us to track down without something reproducible we can run. We won't be able to look into this issue without something reproducible.

As a side note, I am curious why you are using cy.wait() with ms of time to wait. This is typically an anti-pattern.

You would usually want to wait for the conditions to be exactly as needed in order for the elements to exist before continuing (oftentimes this means waiting for an XHR request - see cy.route() + cy.wait()), since waiting 2000ms does not always mean that the element is there. The ms wait would lead to flaky tests and will also slow down the entire suite of tests.

[brian-mann]

This is actually a problem I've seen before on a couple different sites. It's likely due to 3rd party code attempting to instrument the XHR property in a similar way that Cypress does and causes a conflict. Cypress modifies those properties so it can get notifications when XHR's finish (so we can know to stop waiting on them, etc).

We're aware this creates a potential conflict and have a plan to rewrite the entire network layer by moving it outside of Javascript and into the proxy server itself - thus insulating us from needing to modify host objects on the fly.

[frattaro]

There's a 3rd party analytics script, calls to insight.adsrvr.org and times out every time (causes cypress to think the initial page takes 30 seconds). I don't think that's cypress' fault. I'll try removing it from non-prod regions and see if that changes anything.

Otherwise I've been looking into trying to get rid of the usage of cy.wait() but it's a but troublesome. A single page app, I'm having trouble following exactly what's happening, but it reports that elements have 0 width, 0 height when looking immediately after the '(New Url)' event. cy.wait() fixes that.

[frattaro]

Looks like that was it. It was the tag here: https://s.btstatic.com/tag.js

That's just a mess of code, but it loads a BrightTag library (after a quick Googling, they just changed their name to Signal).

[frattaro]

I saw this issue pop up again. I removed the google analytics code from the environment I was testing and haven't seen it since.

Regarding getting rid of cy.wait - I was able to remove them by 1) increasing the default command timeout because the VirtualBox runs slowly (probably a combination of the VM itself not having proper VM settings and that the browser isn't caching anything), and 2) making some modifications to the application so the oauth sign on flows better.
Tests are extremely predictable now. Thank you so much.

I'll leave it up to you guys whether or not you want this bug report hanging around open.

[ghost]

HOW to get rid of this ERROR I have super simple XHR implementation to make calls and this is preventing me from using it.

[brian-mann]

Need a reproducible example to track this down. As of today, nobody has provided one.

[ghost]

I probably know why no one provided one. Problem is that Cypress woks well when you do one call and get answer than just process. BUT for example if you want to get all user repos using github api you can only get certain amount per page and github is giving you info about that in header "Link" so I thought that i can call for next page inside onreadystatechange until I have all pages and than just callback this.

Function collectAllPages is the problematic one.

Here is simple test code:
Remember to put your user/password github has 60 request limit without it
_xhr.setRequestHeader('Authorization', 'Basic '+window.btoa('user:pass'));

export default class BasicRequest { 
    constructor(url) {
        this.__url = url;
        var versions = [
            "MSXML2.XmlHttp.6.0",
            "MSXML2.XmlHttp.5.0",
            "MSXML2.XmlHttp.4.0",
            "MSXML2.XmlHttp.3.0",
            "MSXML2.XmlHttp.2.0",
            "Microsoft.XmlHttp"
        ];
        if (typeof XMLHttpRequest !== 'undefined') {
            this.__xhr =  new XMLHttpRequest();
        }

        for (var i = 0; i < versions.length; i++) {
            try {
                this.__xhr = new ActiveXObject(versions[i]);
                break;
            } catch (e) {}
        }
    }
    
    _send(url, callback, method, data, headers, async) {
        if (async === undefined) {
            async = true;
        }
        var _xhr = this.__xhr;
        this.__xhr.open(method, url, async);

        this.__xhr.onreadystatechange = function () {
            if (_xhr.readyState === 4) {
                var data = {data: _xhr.responseText, headers: _xhr.getResponseHeader('Link')};
                callback(data);
            }
        }
        this.__xhr.validatesSecureCertificate = true;
        if(headers !== null && headers !== undefined) {
            this.__xhr.setRequestHeader(headers.key, headers.val);
        }
        
        this.__xhr.send(data); 
    }
    
    getPageData(txt) {
        var retObj = {};
        if(txt !== null) {
            var link = txt.split(',');

            for(var i = 0; i < link.length; i++) {
                var part =  link[i].split(';');
                var obj = {};
                for(var x = 0; x < part.length; x++) {
                    if(String(part[x].trim()).substring(0,3) === 'rel'){
                        obj.type = this.extractText(part[x].trim());
                    } else {
                        var lnk = part[x].trim();
                        obj.link = lnk.replace( /\?,|[<>]+/gi, '');
                        obj.page = this.getUrlVars(obj.link)['page'];
                    }
                }
                retObj[obj.type] = obj;
            }
        }
        return retObj;
    }
 
    getUrlVars(url) {
        var vars = {};
        var parts = url.replace(/[?&]+([^=&]+)=([^&]*)/gi, function(m,key,value) {
            vars[key] = value;
        });
        return vars;
    }

    extractText(str) {
        var ret = "";
        if(/"/.test(str)) {
            ret = str.match( /"(.*?)"/ )[1];
        } else {
            ret = str;
        }
        return ret;
    }
    
    collectAllPages(url, data, callback){
        var _this = this;
        var _xhr = this.__xhr;
        var json = [];
        this.__xhr.open('GET', this.__url + url, true);
        
        this.__xhr.onreadystatechange = function () {
            if (_xhr.readyState === 4) {
                var arr = JSON.parse(_xhr.responseText);
                for(var i = 0; i < arr.length; i++) {
                    json.push(arr[i]);
                }
                var pages = _this.getPageData(_xhr.getResponseHeader('Link'));
                if(pages['next']) {
                    _xhr.open('GET', pages['next'].link, true);
                    _xhr.validatesSecureCertificate = true;
                    _xhr.setRequestHeader('Authorization', 'Basic '+window.btoa('user:pass'));
                    _xhr.send(data);
                } else {
                    callback(json);
                }
            }
        }
        this.__xhr.validatesSecureCertificate = true;
        this.__xhr.setRequestHeader('Authorization', 'Basic '+window.btoa('user:pass'));
        this.__xhr.send(data); 
    }
    
    get(urlParams, headers, async, callback) {
        this._send(this.__url + urlParams, callback, 'GET', null, headers, async);
    }
    
    post(data, urlParams, headers, async, callback) {
        this._send(this.__url + urlParams, callback, 'POST', data, headers, async);
    }
}

//Just call it something like this:

var_request = new BasicRequest('https://api.github.com/');
this._request.collectAllPages('users/facebook/repos', {}, function(data) {
     var stars_total = 0;
     var project_list = [];
     for(var i = 0; i < data.length; i++) {
         project_list.push({name: data[i].name, stars: data[i].stargazers_count});
         stars_total += data[i].stargazers_count;
     }
});

[ghost]

Is it good example for reproduction? What do you think about it?

[bimimicah]

In our tests, we encountered this error because, like the sample code above, we store 1 XMLHttpRequest instance and re-use it, triggering the bug in Cypress.
(specifically, the bug would always trigger the on the 2nd XHR request)
We worked around the problem by manually 'hacking' the page and re-creating the XMLHttpRequest instance before each XHR request.

For example, from inside a test:

parent.document.getElementById("Your App: '<insert domain here>'").contentWindow['<insert XMLHTTPRequest global instance variable here>'] = new XMLHttpRequest();

[emilkm]

Hi @brian-mann

Need a reproducible example to track this down.

Maybe this will help https://github.com/emilkm/cypress-readystatechange

Let me know if you want me to stick the code somewhere on a server with PHP etc, or you need any information.

I will look at changing amf.js if I decide to continue with cypress, but was curious to know when is the

plan to rewrite the entire network layer by moving it outside of Javascript and into the proxy server itself - thus insulating us from needing to modify host objects on the fly.

likely to be implemented?

[austinfox]

I encountered a similar bug and put out a fix that works for my case. @emilkm, @piotr-tomczyk , @bimimicah, @frattaro, can you guys try pulling my change to check if it works for you as well?

[emilkm]

Thanks @austinfox,

I tried something similar directly in cypress_runner.js (just to see what happens)

return fns[prop] = {fn, enumerable: true, configurable: true};

if (prop == 'onload') { return fns[prop] = {fn, enumerable: true, configurable: true}; } else { return fns[prop] = fn; }

But that didn't quite work for me. I avoid the error, but subsequent calls are not made for a reused xhr. I left it at that, as I can easily avoid the issue by configuring the xhr pool size in my example to 1, which results in xhr objects not being reused.

I don't mind trying your change if you give me the resulting JavaScript.

[austinfox]

I don't mind trying your change if you give me the resulting JavaScript.

@emilkm sorry I don't know what you mean here, were you able to pull the change above (#2299) or are you asking for something different?

[emilkm]

HI @austinfox

I didn't have time looking at modifying the source coffee script and then building. Instead I modified the running dist package, which on Windows goes into
C:\Users\<user>\AppData\Local\Cypress\Cache\3.0.3\Cypress\resources\app\packages\runner\dist\cypress_runner.js

At line 67944 is the code that, I believe your change to the source coffee script will result in the dist java script.

I could try your change by simply editing that file again. Alternatively I would have to build, assuming it is not time consuming.

[lunascat]

can you guys try pulling my change to check if it works for you as well?

@austinfox I've had the same issue, your fix works for me. Thanks!

[austinfox]

@lunascat I'm glad to hear this worked for you. Right now I'm manually patching existing versions of Cypress until this is merged in.

@jennifer-shehane I've tried to create a test for this but have been unsuccessful so far. It requires stubbing out XMLHttpRequest with an existing onreadystatechange fn before cypress loads, but I'm getting race conditions with loading my js to handle this before the cypress js takes over and uses the native XMLHttpsRequest. Is there a chance the Cypress team could help with this? It seems like this solution is unblocking folks and doesn't break anything, the only thing preventing it from being merged right now is a test preventing future regressions as far as I can tell.