alkemics/CancelablePromise

cancelable(...) makes original Promise not handling Error throw

Closed this issue · 6 comments

Deleted user commented

Browser: Chrome 91.0.4472.124

I've noticed throwing error in wrapped (cancelable) promise don't forward it to wrapped promise.catch(...) block
Check example below:

import CancelablePromise, { cancelable } from 'cancelable-promise'

async func getSamplePromise(): Promise<any> {
   throw Error('Error that should be forwarded into promise.catch(...)')
}

let request: CancelablePromise<any> | null = null
const wrappedPromise = cancelable(getSamplePromise())

wrappedPromise.catch(err => {
   //that block is not going to be executed, and Error remains unhandled
})

When original Promise is used it enters .catch(...)

sebdvd commented

Hello @FIFOFridge, thank you for your feedback, I'll have a look at it this week.

sebdvd commented

@FIFOFridge I can't reproduce, I tried this in Chromium 91 and Firefox 90 and it's fine:

<!DOCTYPE html>
<html>
  <body>
    <script type="module">
      import { cancelable } from 'https://unpkg.com/cancelable-promise@4.2.1/esm/CancelablePromise.min.mjs';

      async function getSamplePromise() {
        throw Error('Error that should be forwarded into promise.catch(...)');
      }

      const wrappedPromise = cancelable(getSamplePromise());

      wrappedPromise.catch((err) => {
        console.log('caught:', err.message);
      });
    </script>
  </body>
</html>

Do you have any other information? Such as your cancelable-promise version, or if your script is failing after being processed by your tools (Typescript or Babel)...

Deleted user commented

@alk-abesnard

According to package-lock.json: cancelable-promise: 4.2.1
Tools:

Gatsby CLI version: 3.2.0
Gatsby version: 3.10.2

Furthermore gatsbty (it's almost zero-config tool) is using webpack with babel to process source files, but sadly don't provide webpack config, so i can't dump it and include that.

Language: TypeScript/JavaScript
Framework: React: 17.0.2

I've created minimal repo (using gatsby new) to reproduce error, gatsby CLI is required:
https://github.com/FIFOFridge/cancelable-promise-bug

To reproduce error:

  1. Clone
  2. Instal dependencies using npm install
  3. Run gatsby develop
  4. Enter localhost:8000

I've tested multiple cases, both: tsx, jsx components are behaving same way.

Sample usage:

import React, { useEffect } from 'react'
import { cancelable } from "cancelable-promise"
import { performUtilAsyncTask } from "../utils/sampleUtil"

export const JsComponent = () => {
    const performComponentAynscTask = async () => {
        throw Error("")
    }

    //this code will be executed after component is mounted
    useEffect(() => {
        const externalAsyncTask = cancelable(performUtilAsyncTask())
        const componentAynscTask = cancelable(performComponentAynscTask())

        let isExternalTaskCompleted = false
        let isComponentTaskCompleted = false

        componentAynscTask.catch(() => {
            console.log(`componentAynscTask error handled as expected...`)
        })
        componentAynscTask.finally(() => {
            isComponentTaskCompleted = true
        })

        externalAsyncTask.catch(() => {
            console.log(`externalAsyncTask error handled as expected...`)  
        })
        externalAsyncTask.finally(() => {
            isExternalTaskCompleted = true
        })

        //this code is executed before component unmount
        //react requires async operations cleanup
        return () => {
            if(!isExternalTaskCompleted)
                externalAsyncTask.cancel()

            if (!isComponentTaskCompleted)
                componentAynscTask.cancel()
        }
    }, [])

    return (
        <>
            Hello from JsComponent!
        </>
    )
}

export default JsComponent

Edit: updated syntax highlight and code comments

sebdvd commented

@FIFOFridge ok thanks 👍 I'll try your repo as soon as possible

sebdvd commented

@FIFOFridge I think I understood what's happening here, and it does not seem related to cancelable-promise. Consider this component where p1 is a cancelable promise and p2 a promise:

const IndexPage = () => {
    const asyncTask = async (n) => {
        throw Error(`E${n}`)
    }

    useEffect(() => {
        const p1 = cancelable(asyncTask(1))
        p1.catch((e) => {
            console.log(`p1: catch`, e.message)
        })
        p1.finally(() => {
            console.log(`p1: finally`)
        })

        const p2 = asyncTask(2)
        p2.catch((e) => {
            console.log(`p2: catch`, e.message)
        })
        p2.finally(() => {
            console.log(`p2: finally`)
        })
    }, [])

    return null
}

Notice that the promise and the cancelable promise behave the same, it logs:

p1: catch E1
p1: finally
p2: catch E2
p2: finally
Uncaught (in promise) Error: E1
Uncaught (in promise) Error: E2

Then consider this component, with chained finally:

const IndexPage = () => {
    const asyncTask = async (n) => {
        throw Error(`E${n}`)
    }

    useEffect(() => {
        const p1 = cancelable(asyncTask(1))
        p1.catch((e) => {
            console.log(`p1: catch`, e.message)
        }).finally(() => {
            console.log(`p1: finally`)
        })

        const p2 = asyncTask(2)
        p2.catch((e) => {
            console.log(`p2: catch`, e.message)
        }).finally(() => {
            console.log(`p2: finally`)
        })
    }, [])

    return null
}

It logs:

p1: catch E1
p1: finally
p2: catch E2
p2: finally

Finally it makes sense to me:

let p1 = cancelable(asyncTaskThrowingError())
let p2 = p1.catch(console.log)
let p3 = p1.finally(() => {})

In this case p3 is coming from p1 which doesn't catch anything, finally is executed but the error isn't caught in this branch, the error is caught in p2. But if you do:

let p1 = cancelable(asyncTaskThrowingError())
let p2 = p1.catch(console.log)
let p3 = p2.finally(() => {})

Now p3 is coming from p2 which do catch the error, so finally is executed and there is no uncaught error. Does it make sense to you?

Deleted user commented

@alk-sdavid ahhh it makes a lot of sense, I didin't knew that blocks have to be chained and looks like 'default' promise require exacly same way to handle, so far I was sure each promise instance can share it's state between multiple non-chained blocks, I have to investigate that deeply

Sorry for wasting your time and thanks for helping me to figure it out 😄