tapable icon indicating copy to clipboard operation
tapable copied to clipboard

[Bug] Async hook doesn't catch error when reject a falsy value

Open peakchen90 opened this issue 3 years ago • 6 comments

Hi, I seem to have found a bug:

Environment

  • Tapable: v2.2.1
  • Node: v16.16.0
  • OS: MacOS 12.5

Case

const { AsyncSeriesHook } = require('tapable');

const hook = new AsyncSeriesHook();
hook.tapPromise('A', () => {
  return Promise.reject(0); // reject a falsy value
});
hook.tapPromise('B', () => {
  console.log('some thing');
});

hook.callAsync((err) => {
  if (err) {
    console.error('Error:', err);
  } else {
    console.log('Success');
  }
});

Expected output: Error: 0

But the actual output: Success

peakchen90 avatar Aug 04 '22 17:08 peakchen90

Hm, why you pass 0 in reject?

alexander-akait avatar Aug 10 '22 11:08 alexander-akait

This is just an example, my real case is: Project.reject() , reject a undefined

peakchen90 avatar Aug 10 '22 12:08 peakchen90

Can you try to use Promise.reject(new Error("test"))?

alexander-akait avatar Aug 10 '22 12:08 alexander-akait

Of course, I can somehow ignore this case, but I think this can be made more robust. I don't use hooks directly, and I expose them to others, so I have to fix. Below shows how I fixed it. and I hope the official can fix it

function _fixTapablePromiseHook(hook: Hook<any, any>) {
  hook.intercept({
    register: (tapInfo) => {
      const originalFn = tapInfo.fn;
      if (tapInfo.type === 'promise') {
        tapInfo.fn = async function (...args: any[]) {
          try {
            return await originalFn.apply(tapInfo, args);
          } catch (err) {
            if (!err) err = new Error();
            throw err;
          }
        };
      }
      return tapInfo;
    },
  });
}

peakchen90 avatar Aug 10 '22 13:08 peakchen90

hm, looks good, feel free to send a PR

alexander-akait avatar Aug 10 '22 13:08 alexander-akait

Hi, I would like to take this issue !!

Solo-steven avatar Oct 10 '22 11:10 Solo-steven