Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
peakchen90 opened this issue Aug 4, 2022 · 6 comments
Open

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

peakchen90 opened this issue Aug 4, 2022 · 6 comments

Comments

@peakchen90
Copy link

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

@alexander-akait
Copy link
Member

Hm, why you pass 0 in reject?

@peakchen90
Copy link
Author

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

@alexander-akait
Copy link
Member

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

@peakchen90
Copy link
Author

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
Copy link
Member

hm, looks good, feel free to send a PR

@Solo-steven
Copy link

Hi, I would like to take this issue !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants