avadev/AvaTax-REST-V2-JS-SDK

withTimeout() causes Node to hang

michaelsinghurse opened this issue · 6 comments

I'm calling the taxRatesByAddress method and am successfully getting back a tax rates object, but my application hangs every time. Something is preventing Node from closing.

I used the wtfnode package to investigate. This is the output I received:

^C[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
- Timers:
  - (1200000 ~ 20 min) (anonymous) @ /.../node_modules/avatax/lib/utils/withTimeout.js:9

The withTimeout function returns Promise.race(), with two promises in the iterable. The first one is a timer for 20 minutes. It seems to me that this timer is keeping the application open.

I think this could be fixed if AbortController (abort-controller) is used instead of Promise.race, the fetch library would have to be switched to node-fetch do that since I don't think the current fetch library supports it.

const controller = new AbortController();
const signal = controller.signal;
const options = {
   signal,
   method,
   headers: {
     ...fetchHeaders,
     ...headers,
   },
 };
 let timedOut = false;
 const timeout = setTimeout(() => {
     timedOut = true;
     controller.abort();
  },12000)
return fetch(url, options)
   .catch(err => {
    clearTimeout(timeout);
     if (timedOut) {
       throw Object.assign(new Error(`Request: ${fetchUrl} timed out after ${timeout} milliseconds`), { type: 'timeout' });
     }
     throw err;
   });

Ben, could you do something like this, which doesn't require any additional dependencies? It's not too elegant but might get the job done.

function withTimeout(msecs, promise) {
  return new Promise((resolve, reject) => {
    let timeoutId = setTimeout(() => {
      reject(new Error('timeout'));
    }, msecs);

    promise
      .then(result => {
        clearTimeout(timeoutId);
        resolve(result);
      })
      .catch(error => {
        reject(error);
      });
  });
}

Oh, yes. I misread the original issue and missed that you were getting a resolution of the tax rates object.

Think my example is fixing a bug you're not experiencing.

Sounds good, Ben. One modification to what I proposed above. I think you'd want to clear the timeout whether the promise resolves or rejects, and so it could go in the finally method.

function withTimeout(msecs, promise) {
  return new Promise((resolve, reject) => {
    let timeoutId = setTimeout(() => {
      reject(new Error('timeout'));
    }, msecs);

    promise
      .then(result => {
        resolve(result);
      })
      .catch(error => {
        reject(error);
      })
      .finally(() => {
        clearTimeout(timeoutId);
      });
  });
}

Any updates here? I still use version 20.6.0 and cannot update it because of this core issue.

This has been resolved. Please upgrade to the latest version of SDK.
Thanks