MONEI / Shopify-api-node

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

Uncatchable exception thrown #636

Closed Klem3n closed 11 months ago

Klem3n commented 11 months ago

When shopify throws an error for a request it can not be caught.

Versions:

shopify-api-node: v3.12.6 node: v18.15.0 Windows 10 Pro

Reproduction steps:

// Get metafields of an order that does not exist.

const shopify = new Shopify({
        accessToken: ACCESS_TOKEN,
        shopName: SHOP_NAME,
        apiVersion: API_VERSION,
        autoLimit: true,
      });

await shopify.metafield.list({
            metafield: { owner_resource: 'order', owner_id: .1 },
            namespace: 'xxx',
            key: 'yyy',
          });

// Got will throw a 404 error that will crash the program.
// It can not be caught by wrapping the call with a try/catch

Issue:

The issue lies on this line - https://github.com/MONEI/Shopify-api-node/blob/c109341f0921a300e93d028d8f2dbfb1abe74bf3/index.js#L194C43-L194C43

It only handles a successful promise, but does not catch an exception.

The error thrown by got should be handled. (.catch((err) => { console.error(err); throw err; //? }))

lpinca commented 11 months ago

It only handles a successful promise, but does not catch an exception.

This is not correct. The referenced line returns a promise which you are awaiting. You have to wrap await in a try...catch statement and handle the error.

lpinca commented 11 months ago

Here is an example.

$ cat test.mjs    
function foo() {
  return new Promise(function (resolve, reject) {
    setTimeout(reject, 100, new Error('Oops'));
  });
}

try {
  await foo();
} catch (e) {
  console.log('Caught error: %s', e.message);
}
$ node test.mjs 
Caught error: Oops

I'm closing this as answered.