MONEI / Shopify-api-node

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

"401 Unauthorized" (or 200 OK with error HTML) because shopify-api-node passes `pathname` to `got` instead of `path` #397

Closed emk closed 4 years ago

emk commented 4 years ago

Thank you for maintaining shopify-api-node!

This is closely related to the problems described in https://github.com/MONEI/Shopify-api-node/issues/371.

Versions:

What happens

The following code fails with some mix of timeouts, 401 errors, and similar problems, depending on exactly what versions of Node, shopify-api-node and got I use:

    const client = new Shopify({
      apiVersion: '2020-04',
      shopName: shop,
      accessToken,
    });
    const shopInfo = await client.shop.get()

This appears to be caused by shopify-api-node passing pathname to got when got wants path instead.

Diagnosis

If I invoke got manually using the following:

got({
  "pathname":"/admin/api/2020-04/shop.json",
  "hostname":"example.myshopify.com",
  "protocol":"https:"
}, {
  "headers": {
    "User-Agent":"shopify-api-node/3.3.2",
    "X-Shopify-Access-Token":"shpat_TOKEN_HERE",
    "Accept": "application/json"
  },
  "timeout":60000,
  "responseType":"json",
  "retry":0,
  "method":"GET"
})

...it fails with:

{
  message: 'Response code 401 (Unauthorized)',
  host: undefined,
  hostname: 'example.myshopify.com',
  method: 'GET',
  path: '',
  statusCode: 401,
  statusMessage: 'Unauthorized'
}

If we leave out "Accept": "application/json", it will instead receive a "200 OK" message containing the home page and a log in form.

To fix this, I can replace:

"pathname":"/admin/api/2020-04/shop.json",

...with:

"path":"/admin/api/2020-04/shop.json",

...and everything will work fine.

But shopify-api-node is passing pathname, not path, and so it fails.

seamusabshere commented 4 years ago

makes you wish javascript had real kwargs...

lpinca commented 4 years ago

From https://github.com/sindresorhus/got#options

Note: Legacy URL support is disabled. options.path is supported only for backwards compatibility. Use options.pathname and options.searchParams instead. options.auth has been replaced with options.username & options.password.

lpinca commented 4 years ago

Also I can't reproduce:

$ node -v
v14.5.0
$ cat gh-397.js 
'use strict';

const Shopify = require('shopify-api-node');

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

(async function () {
  const shop = await shopify.shop.get();
  console.log(shop);
})().catch(console.error);
$ node gh-397.js 
{
  id: 19563841,
  name: 'quuz',
  email: 'al.bino@rocketmail.com',
  domain: 'quuz.myshopify.com',
  province: 'Roma',
  country: 'IT',
  address1: 'Via Roma 1',
  zip: '12345',
  city: 'Rome',
  source: null,
  phone: '',
  latitude: 42.137833,
  longitude: 12.380085,
  primary_locale: 'en',
  address2: null,
  created_at: '2017-04-26T03:47:03-04:00',
  updated_at: '2020-07-06T18:31:41-04:00',
  country_code: 'IT',
  country_name: 'Italy',
  currency: 'EUR',
  customer_email: 'al.bino@rocketmail.com',
  timezone: '(GMT-05:00) Eastern Time (US & Canada)',
  iana_timezone: 'America/New_York',
  shop_owner: 'Al Bino',
  money_format: '€{{amount}}',
  money_with_currency_format: '€{{amount}} EUR',
  weight_unit: 'kg',
  province_code: 'RM',
  taxes_included: true,
  tax_shipping: null,
  county_taxes: true,
  plan_display_name: 'Development',
  plan_name: 'affiliate',
  has_discounts: false,
  has_gift_cards: false,
  myshopify_domain: 'quuz.myshopify.com',
  google_apps_domain: null,
  google_apps_login_enabled: null,
  money_in_emails_format: '€{{amount}}',
  money_with_currency_in_emails_format: '€{{amount}} EUR',
  eligible_for_payments: true,
  requires_extra_payments_agreement: false,
  password_enabled: true,
  has_storefront: true,
  eligible_for_card_reader_giveaway: false,
  finances: true,
  primary_location_id: 42043015,
  cookie_consent_level: 'implicit',
  visitor_tracking_consent_preference: 'allow_all',
  force_ssl: true,
  checkout_api_supported: false,
  multi_location_enabled: true,
  setup_required: false,
  pre_launch_enabled: false,
  enabled_presentment_currencies: [ 'EUR' ]
}
emk commented 4 years ago

Also I can't reproduce:

$ node -v
v14.5.0

This failed for me under Node 12.18.2, which is currently an LTS release. (See above for the list of versions I used.) I was also able to reproduce it under Node 12.14.1. Depending on the exact versions of Node and got I tested with, this error could manifest as a 401, a parse error, or a timeout.

I specifically verified that shopify-api-node 3.3.2 running on Node 12.18.2 was passing pathname to got, but that got 11.5.1 on Node 12.18.2 ignored pathname and that it only worked if received path. I know the docs say pathname should work, but I actually called got manually with pathname and it raised this error:

{
  message: 'Response code 401 (Unauthorized)',
  host: undefined,
  hostname: 'example.myshopify.com',
  method: 'GET',
  path: '',
  statusCode: 401,
  statusMessage: 'Unauthorized'
}

When I switched to passing path to got, everything worked fine.

There's a possibility that this might also be related to #386 or #371, which look suspiciously similar.

Interestingly, your gh-397.js script justed worked fine under 12.14.1. So there's something weird going on at the interface between shopify-api-node and got, but it may only trigger in the presence of certain other modules? (But note that a fresh Node with let got = require('got'); got({pathname: ... }) reproducibly failed for me yesterday on multiple Node 12.x versions.)

Since we only need a couple of Shopify API endpoints, we've switched from shopify-api-node to node-fetch. I wish I could help you more with this, because it's a genuinely weird bug. I reported this issue just in case it gave you any further clues.

lpinca commented 4 years ago

@emk can you run npm ls agent-base and show the result? I think the issue is caused by an old version of the agent-base module that monkey patches https.request().

raffi1300 commented 4 years ago

@emk can you run npm ls agent-base and show the result? I think the issue is caused by an old version of the agent-base module that monkey patches https.request().

+-- pm2@4.4.0 | -- @pm2/agent@1.0.4 |-- proxy-agent@3.1.1 | +-- agent-base@4.3.0 | +-- http-proxy-agent@2.1.0 | | -- agent-base@4.3.0 | +-- https-proxy-agent@3.0.1 | |-- agent-base@4.3.0 | +-- pac-proxy-agent@3.0.1 | | -- agent-base@4.3.0 |-- socks-proxy-agent@4.0.2 | -- agent-base@4.2.1 -- puppeteer@4.0.1 -- https-proxy-agent@4.0.0 -- agent-base@5.1.1

Not the OP, but I also have this issue with the newer packages. running the latest 2.x version gives me no issues

lpinca commented 4 years ago

I'm closing as the issue is caused by agent-base@<=4. There is not much we can do.