MONEI / Shopify-api-node

Node Shopify connector sponsored by MONEI
https://monei.com/shopify-payment-gateway/
MIT License
945 stars 278 forks source link

Network request with shopify.product.list including a query string creates an invalid request #539

Closed JordanDysart closed 2 years ago

JordanDysart commented 2 years ago

When trying to query shopify for a list of products and the templates that they were using I was receiving an error as a response from Shopify.

This request

shopify.product.list({
        fields: 'template_suffix, handle, title',
        limit: '250',
        page: pageNum
      })

Would result in this error.

  name: 'HTTPError',
  code: 'ERR_NON_2XX_3XX_RESPONSE',

I resolved the issue by changing this function found in file Shopify-api-node/mixins/base.js.

   * Builds the request URL.
   *
   * @param {Number|String} [id] Record ID
   * @param {Object} [query] Query parameters
   * @return {Object} URL object
   * @private
   */
  buildUrl(id, query) {
    id || id === 0 || (id = '');

    let pathname = '/admin';

    if (this.shopify.options.apiVersion) {
      pathname += `/api/${this.shopify.options.apiVersion}`;
    }

    pathname += `/${this.name}/${id}`;
    pathname = pathname.replace(/\/+/g, '/').replace(/\/$/, '') + '.json';

    const url = { pathname, ...this.shopify.baseUrl };

    if (query) {
      url.search = '?' + qs.stringify(query, { arrayFormat: 'brackets' });
    }

    return url;
  }
};

The line url.search = '?' + qs.stringify(query, { arrayFormat: 'brackets' }); was updated to url.query = '?' + qs.stringify(query, { arrayFormat: 'brackets' });. (i.e search changed to query).

Would it make sense to create a pull request to change this? I'm not 100% familiar with the entire API, so I cannot confirm if this issue pops up while using other endpoints.

lpinca commented 2 years ago

I cannot reproduce the issue:

$ cat gh-539.mjs 
import Shopify from 'shopify-api-node';

const shopify = new Shopify({
  shopName: 'quuz',
  apiVersion: '2021-01',
  apiKey: 'b68c3a4bf44cd240511e5f4114f0be4d',
  password: '2d170167fc62551920ca60fe40a5e548'
});

const products = await shopify.product.list({
  fields: 'template_suffix, handle, title'
});

console.log(products);
$ node gh-539.mjs 
[]

Also, it does not make much sense. url.search and url.query are two different things.

$ node
Welcome to Node.js v18.2.0.
Type ".help" for more information.
> url.parse('http://example.com/?foo=bar')
Url {
  protocol: 'http:',
  slashes: true,
  auth: null,
  host: 'example.com',
  port: null,
  hostname: 'example.com',
  hash: null,
  search: '?foo=bar',
  query: 'foo=bar',
  pathname: '/',
  path: '/?foo=bar',
  href: 'http://example.com/?foo=bar'
}

See also https://github.com/sindresorhus/got/blob/5e17bb748c260b02e4cf716c2f4079a1c6a7481e/source/core/utils/url-to-options.ts#L31.

JordanDysart commented 2 years ago

@lpinca thanks for pointing that out. I've been tricked by my own inexperience. The got docs were very helpful.

It looks like my issue is in my own code, I've been updating some old tools that were written before Shopify switched to cursor based pagination. page is no longer supported.

https://shopify.dev/api/usage/pagination-rest

And! you already covered in the docs that I skimmed past.

https://github.com/MONEI/Shopify-api-node#pagination

Thanks, looks like this isn't an issue.

lpinca commented 2 years ago

Ok, I'm closing this then.