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

Question: Why use .catch on the promise #9

Closed
terenced opened this issue May 12, 2021 · 4 comments
Closed

Question: Why use .catch on the promise #9

terenced opened this issue May 12, 2021 · 4 comments

Comments

@terenced
Copy link

terenced commented May 12, 2021

I am curious why this change was introduced.

// Original implementation  
export const until = async <DataType = unknown, ErrorType = Error>(promise: () => Promise<DataType>): Promise<[ErrorType, DataType]> => {
  try {
    const data = await promise()
    return [null, data]
  } catch (error) {
    return [error, null]
}
// Current implementation 
export const until = async <DataType = unknown, ErrorType = Error>(promise: () => Promise<DataType>): Promise<[ErrorType, DataType]> => {
  try {
   const data = await promise().catch((error) => { // .catch added
      throw error
    })
    return [null, data]
  } catch (error) {
    return [error, null]
}

Is there a benefit of calling .catch on the promise? It is my understanding that the catch block would handle throws?

@newpost
Copy link

newpost commented Aug 5, 2021

@terenced can you tell me why?thank you

@kettanaito
Copy link
Member

The .catch() block was added to propagate unhandled exceptions to the library.

When there's an exception thrown during a Promise, it will not be caught by a try/catch block around it:

try { new Promise(() => { throw new Error('Hey') }) } catch (e) { console.log(e) }

The console.log will never be called. The thrown exception will be printed as an UnhandledPromiseRejectionWarning.

Flow-wise, that is still an unsuccessful promise execution, so this library should execute the return [error, null] closure for the dependent code to handle it like an error. Adding an explicit .catch() statement to the given promise catches any errors occurring in that promise.

This solution also doesn't interfere with you defining your custom .catch(), as catch doesn't have an accumulative nature. If you've defined your custom .catch() block, it will get executed first, and the library's .catch() will not, unless you throw another exception in your own catch block.

new Promise(() => { throw new Error('Hey') }).catch(e => console.log('Custom', e)).catch(e => console.log('Library', e))

@terenced
Copy link
Author

terenced commented Aug 6, 2021

@kettanaito Thank you!

@sbking
Copy link

sbking commented Dec 6, 2022

@kettanaito I think this might be incorrect - await should throw on promise rejection

In your example you're just missing an await - add it and console.log is called:

try { await new Promise(() => { throw new Error('Hey') }) } catch (e) { console.log(e) }

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

4 participants