webpack / tapable

Just a little module for plugins.
MIT License
3.71k stars 393 forks source link

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

Open peakchen90 opened 1 year ago

peakchen90 commented 1 year ago

Hi, I seem to have found a bug:

Environment

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

alexander-akait commented 1 year ago

Hm, why you pass 0 in reject?

peakchen90 commented 1 year ago

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

alexander-akait commented 1 year ago

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

peakchen90 commented 1 year ago

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;
    },
  });
}
alexander-akait commented 1 year ago

hm, looks good, feel free to send a PR

Solo-steven commented 1 year ago

Hi, I would like to take this issue !!