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

cy.wait() does not resolve when the XHR contains static resource-like text in the XHR's query params or hash (like .js, .html, .css) #7280

Closed
hudsonsdad opened this issue May 8, 2020 · 10 comments · Fixed by #7742
Labels
pkg/driver This is due to an issue in the packages/driver directory type: bug

Comments

@hudsonsdad
Copy link

hudsonsdad commented May 8, 2020

Current behavior:

Discovered this while trying to test for ad requests for my employer. When a URL with file extension, like .html, and a question mark (for a query string parameter) is visited, the request that is listened for is no longer detected. Remove the question mark, and the request is detected.

Desired behavior:

Before promoting a build in production, we test the build using a query-string parameter, so the wish is for this to work when a query-string parameter is present in the visited URL.

Test code to reproduce

describe("Oregon Live Ads", () => {
  it("test cy.route()", () => {
    cy.viewport(1500, 1024);

    Cypress.on('uncaught:exception', () => {
      return false;
    });

    cy.server();

    cy.route({
      url: /gampad\/ads\?/,
    }).as("waitForAd");

    cy.visit(
      "https://www.oregonlive.com/news/2019/12/vaping-related-lung-illness-epidemic-has-likely-peaked-feds-say.html"
      , {qs: {answer: "42"}}  // When this is commented out, the test passes.
    );

    cy.wait("@waitForAd").then((req) => {
      expect(true).to.equal(true);
    });
  });
});

Versions

Cypress Version: Cypress 4.2.0
OS: MacOS Catalina 10.15.2
Browser: Default Chrome browser that is built into Cypress

@jennifer-shehane
Copy link
Member

Comparing the screenshots of commands when the test fails versus passes shows that the request for the gampad/ads XHR is not made when the site is visited with query params present.

Have you checked that this behavior actually works within the website (outside of Cypress) when query params are present?

with query params

Screen Shot 2020-05-11 at 5 53 47 PM

without query params

Screen Shot 2020-05-11 at 5 54 07 PM

@cypress-bot cypress-bot bot added the stage: awaiting response Potential fix was proposed; awaiting response label May 11, 2020
@hudsonsdad
Copy link
Author

hudsonsdad commented May 11, 2020

Comparing the screenshots of commands when the test fails versus passes shows that the request for the gampad/ads XHR is not made when the site is visited with query params present.

Have you checked that this behavior actually works within the website (outside of Cypress) when query params are present?

with query params

Screen Shot 2020-05-11 at 5 53 47 PM

without query params

Screen Shot 2020-05-11 at 5 54 07 PM

@jennifer-shehane, yes, when visiting the URL with query params, the request for ads is still an XHR request w/ gampad/ads in the route.

image

@hudsonsdad
Copy link
Author

hudsonsdad commented May 28, 2020

@jennifer-shehane, is there anything that I can do to help for this? The problem occurs whenever there is a combination of a file extension in the url plus any query-string parameter, .html?anything=true. This can be reproduced by testing against sites that use .html file extensions in URLs.

@hudsonsdad hudsonsdad changed the title Any query-string parameter prevents route from matching request Any query-string parameter prevents route from matching request when file extension is in URL, like .html May 29, 2020
@CypressCecelia CypressCecelia added stage: needs investigating Someone from Cypress needs to look at this and removed stage: awaiting response Potential fix was proposed; awaiting response labels May 29, 2020
@hudsonsdad
Copy link
Author

hudsonsdad commented May 29, 2020

Just to rule out the possibility that something within my employer's articles might be causing an issue, the same test was run against CNN.com and BBC.com articles, and the results are the same.

When the same test is run against a CNN article, because CNN uses an html file extension in their content URLs, if a query string is present, ad requests are not detected. When the query string is removed, the ad requests are detected.

With Query String
image

describe("CNN Ads", () => {
  it("test cy.route()", () => {
    cy.viewport(1500, 1024);

    Cypress.on('uncaught:exception', () => {
      return false;
    });

    cy.server();

    cy.route({
      url: /gampad\/ads\?/,
    }).as("waitForAd");

    cy.visit(
      "https://www.cnn.com/2020/05/29/politics/george-floyd-protests-american-racism/index.html"
      , {qs: {answer: "42"}}  // When this is removed, the test passes.
    );

    cy.wait("@waitForAd").then((req) => {
      expect(true).to.equal(true);
    });
  });
});

The same CNN article is tested without a query string and the test passes.
image

describe("CNN Ads", () => {
  it("test cy.route()", () => {
    cy.viewport(1500, 1024);

    Cypress.on('uncaught:exception', () => {
      return false;
    });

    cy.server();

    cy.route({
      url: /gampad\/ads\?/,
    }).as("waitForAd");

    cy.visit(
      "https://www.cnn.com/2020/05/29/politics/george-floyd-protests-american-racism/index.html"
    );

    cy.wait("@waitForAd").then((req) => {
      expect(true).to.equal(true);
    });
  });
});

The BBC does not have an html file extension in their URLs, so the test passes regardless of whether or not a query parameter is present.

image

describe("BBC Ads", () => {
  it("test cy.route()", () => {
    cy.viewport(1500, 1024);

    Cypress.on('uncaught:exception', () => {
      return false;
    });

    cy.server();

    cy.route({
      url: /gampad\/ads\?/,
    }).as("waitForAd");

    cy.visit(
      "https://www.bbc.com/news/world-middle-east-52847175"
      , {qs: {answer: "42"}}  // The test passes for the BBC with and without this line of code.
    );

    cy.wait("@waitForAd").then((req) => {
      expect(true).to.equal(true);
    });
  });
});

@jennifer-shehane
Copy link
Member

This fails regardless of the qs option being passed to the URL (just to rule out an issue with the qs option itself). So if you just type the query string in the cy.visit() url directly - it will also fail.

I didn't see any evidence of this being a regression.

spec.js

it('test cy.route()', () => {
  cy.on('uncaught:exception', () => {
    return false;
  });
  cy.viewport(1500, 1024)
  cy.server()
  cy.route(/gampad\/ads\?/).as('waitForAd')
  cy.visit('https://www.oregonlive.com/news/2019/12/vaping-related-lung-illness-epidemic-has-likely-peaked-feds-say.html'
  , { qs: { answer: 42 }} // comment out this line to pass test
  )
  cy.wait('@waitForAd')
})

You can see that the request is made in both situations, just one does not get picked up by our cy.wait(). So this is not affecting the behavior of the site itself, just making it so you cannot use cy.wait() for this specific request.

@hudsonsdad
Copy link
Author

hudsonsdad commented Jun 1, 2020

@jennifer-shehane - yes, this also fails when a question mark, alone, is at the end of the visited URL with a file extension.

@cypress-bot cypress-bot bot added stage: work in progress and removed stage: needs investigating Someone from Cypress needs to look at this labels Jun 17, 2020
@jennifer-shehane jennifer-shehane added the pkg/driver This is due to an issue in the packages/driver directory label Jun 17, 2020
@jennifer-shehane
Copy link
Member

jennifer-shehane commented Jun 18, 2020

We've identified why this is happening in your case.

The presence of query params in the visited URL essentially changes some of the query params in the request made by the securepubads ad.

Cypress inherently hides non XHR resources from the Command Log and from being recognized by cy.route()/cy.wait(). We do this by looking at the request URL and ignoring URLs that seemingly are static resources (containing things like .html, .js, .css, etc.

// Actual REGEX we use to ignore static resources
/\.(jsx?|coffee|html|less|s?css|svg)(\?.*)?$/

The XHR request you wanted to listen to in your case contained some of the keywords we're looking to ignore in the query params of the request (only matching when the main URL contained query params) like below:

https://securepubads.g.doubleclick.net/gampad/ads?...&url=https://www.oregonlive.com/news/2019/12/vaping-related-lung-illness-epidemic-has-likely-peaked-feds-say.html

I've opened a pull request to first remove anything from the query params and the hash before Cypress looks for resources to ignore. #7742 This fixes your issue.

Reproducible example

A better reproducible example of the original issue:

it('test cy.route()', () => {
  cy.server()
  cy.route(/url-with-query-param/, { foo: 'bar' }).as('getQueryParam')
  cy.visit('https://example.com')
  cy.window().then((win) => {
    const xhr = new win.XMLHttpRequest()
    // our XHR has `.js` in it, which we normally ignore as XHR resource
    xhr.open('GET', '/url-with-query-param?resource=foo.js')
    xhr.send()
  })
  cy.wait('@getQueryParam')
})

@jennifer-shehane jennifer-shehane changed the title Any query-string parameter prevents route from matching request when file extension is in URL, like .html cy.wait() does not resolve when the XHR contains static resource-like text in the XHR's query params or hash (like .js, .html, .css) Jun 18, 2020
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Jun 18, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 19, 2020

The code for this is done in cypress-io/cypress#7742, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Jun 19, 2020
@hudsonsdad
Copy link
Author

Thank you very much, @jennifer-shehane!

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 23, 2020

Released in 4.9.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg/driver This is due to an issue in the packages/driver directory type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants