Array and Objects in "qs" params incorrectly parsed in cy.request() after updated to the latest Cypress version

[AlexanderTunick]

Current behavior

Current behavior After updating Cypress version to the latest some of my tests started falling. The tests that affected contain cy.request() with qs wich is represented like

{
        limit: 10,
        page: 1,
        where: {
            commission: {filterCommissionType: commissionType},
            types: {klickly_orders: 1, shopify_orders: 0},
            date: {
                startDate: startDate || moment().subtract(1, 'day').toISOString(),
                endDate: endDate || moment().toISOString()
            }
        },
        order: [ 'processed_at', 'desc' ]
    };

When the request is being sent I see this code in the console: "https://brands-dashboard-api.klickly-dev.com/orders?limit=10&page=1&where=%5Bobject%20Object%5D&order=processed_at%2Cdesc";

But it should be: "https://brands-dashboard-api.klickly-dev.com/orders?limit=10&page=1&where%5Btypes%5D%5Bklickly_orders%5D=1&where%5Btypes%5D%5Bshopify_orders%5D=0&where%5Bcommission%5D%5BfilterCommissionType%5D=all&where%5Bdate%5D%5BstartDate%5D=2023-08-05T00%3A00%3A00.000Z&where%5Bdate%5D%5BendDate%5D=2023-09-04T23%3A59%3A59.999Z&order%5B0%5D=processed_at&order%5B1%5D=desc";

My software details: Cypress v13.1.0 Chrome instance v116 Macbook - Apple M2 Max, Ventura 13.0 Node v16.15.0

Desired behavior

No response

Test code to reproduce

// support.cy.js

const orderApiQS = (data) => {
    const {startDate = null, endDate = null, commissionType = 'all'} = data || {};
    return {
        limit: 10,
        page: 1,
        where: {
            commission: {filterCommissionType: commissionType},
            types: {klickly_orders: 1, shopify_orders: 0},
            date: {
                startDate: startDate || moment().subtract(1, 'day').toISOString(),
                endDate: endDate || moment().toISOString()
            }
        },
        order: [ 'processed_at', 'desc' ]
    };
};

Cypress.Commands.add('apiGetOrdersList', () => {
    return cy.request({
        method: 'GET',
        url: 'https://brands-dashboard-api.klickly-dev.com/orders',
        qs: orderApiQS(),
        timeout: 60000
    });
});

Cypress.Commands.add('loginBrandDashboard', (email = brandEmail, password = brandPass) => {
    cy.session(email, () => {
        cy.request({
            method: 'POST',
            url: 'https://brands-dashboard-api.klickly-dev.com/auth/login',
            body: {
                email,
                password
            },
            timeout: 60000
        }).then((resp) => {
            expect(resp.status, 'Brand Dashboard login').to.eq(200);
        });
    });
});

// file.cy.js

it.('should ', () => {
        cy.loginBrandDashboard('klickly.e2e+development@gmail.com');
        cy.apiGetOrdersList();
    });

Cypress Version

13.1.0

Node version

16.15/0

Operating System

MacOS Ventura 13.0

Debug Logs

No response

Other

No response

[AlexanderTunick]

Can someone review it?

[nagash77]

Hi @AlexanderTunick what version are you upgrading from?

[AlexanderTunick]

@nagash77 went through several versions but the last one was 12.17.4

[AtofStryker]

@AlexanderTunick @nagash77 I was able to take a look at this and put the code in a reproduction repository https://github.com/AtofStryker/cypress-issue-27745.

It looks like the bug was introduced in 12.0.0 in commit 13190c49bb5c6f86f475fd5d018789efe965d15d. The original PR is https://github.com/cypress-io/cypress/pull/20302, where the qs option is now added to the overall Url. The problem is url-parse doesn't know how to serialize objects in query strings, hence the output you are seeing. We should be able to do something like this with the mergeUrlWithParams from that commit (or only in the request code to reduce the blast radius) to fix the problem.

static mergeUrlWithParams (url, params) {
  url = new UrlParse(url, null, true)
  const mergedQuery = _.merge(url.query || {}, params)
  const serializedQuery = qs.stringify(mergedQuery)

  url.set('query', serializedQuery)

  return url.toString()
}

[AlexanderTunick]

@AtofStryker thank you, is it possible to give any ETA for this bug? I've already worked around it but interestingly when to give myself a reminder to revert to the original code.

[AtofStryker]

@AlexanderTunick no ETA yet, but when the team does pick up this issue we will keep you updated!

[S-Tornqvist]

@AtofStryker @nagash77 could I start working on this? I'm thinking of using npm qs (looks like what you're doing in the example). Might be some higher level considerations for limiting nested depth and such, but the implementation looks straight forward to me.

I'm new here, so a bit unsure what your stance on new dependencies is. Could probably be solved with some custom independent solution also

[AtofStryker]

@AtofStryker @nagash77 could I start working on this? I'm thinking of using npm qs (looks like what you're doing in the example). Might be some higher level considerations for limiting nested depth and such, but the implementation looks straight forward to me.

I'm new here, so a bit unsure what your stance on new dependencies is. Could probably be solved with some custom independent solution also

@S-Tornqvist Of course! We are always open to contributions. My guess is we would need to pin qs as a dependency in packages/server and try updating mergeUrlWithParams to code similar to above, add a test in the driver for request.cy.js and create a pull request. When we run the changes against CI we should be able to see what possible regressions there are with cy.visit() and other methods that are relying on mergeUrlWithParams.

[S-Tornqvist]

@AtofStryker thanks for the instructions. Much appreciated