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

Deno.test cannot handle the simplest napi async function #23253

Open
loynoir opened this issue Apr 6, 2024 · 2 comments
Open

Deno.test cannot handle the simplest napi async function #23253

loynoir opened this issue Apr 6, 2024 · 2 comments
Labels
bug Something isn't working node compat node native extension related to the node-api (.node) testing related to deno test and coverage

Comments

@loynoir
Copy link

loynoir commented Apr 6, 2024

bug

Deno.test cannot handle the simplest napi async function

  • nopAsync: sometimes success, sometimes hangs forever

  • sleepAsync: Promise resolution is still pending but the event loop has already resolved.

  • fourtyTwoAsync: Promise resolution is still pending but the event loop has already resolved.

version

deno is latest

$ deno --version
deno 1.42.1 (release, x86_64-unknown-linux-gnu)
v8 12.3.219.9
typescript 5.4.3

deps are latest

[dependencies]
napi = { version = "2.16.1", features = ["tokio_rt", "napi9"] }
napi-derive = "2.16.1"
tokio = { version = "1.37.0", features = ["full"] }

reproduce

rust napi code

#[napi_derive::napi]
pub async fn nop_async() {}

#[napi_derive::napi]
pub async fn sleep_async(delay: u32) {
    napi::tokio::time::sleep(napi::tokio::time::Duration::from_millis(delay.into())).await;
}

#[napi_derive::napi]
pub async fn fourty_two_async(delay: u32) -> u32 {
    napi::tokio::time::sleep(napi::tokio::time::Duration::from_millis(delay.into())).await;

    42
}

reproduce.ts

import { fourtyTwoAsync, nopAsync, sleepAsync } from '@my/napi'

switch (Deno.env.get('TEST')) {
  case nopAsync.name:
    Deno.test(nopAsync.name, async () => {
      await nopAsync()
    })
    break
  case sleepAsync.name:
    Deno.test(sleepAsync.name, async () => {
      await sleepAsync(100)
    })
    break
  case fourtyTwoAsync.name:
    Deno.test(fourtyTwoAsync.name, async () => {
      const expected = 42
      const actual = await fourtyTwoAsync(100)

      if (actual !== expected) throw new Error()
    })
    break
  default:
    throw new Error()
}

actual

  • nopAsync: sometimes success, sometimes hangs forever
$ TEST=nopAsync deno test -A -c ./deno.jsonc --parallel --no-check --no-lock --reload=file:///$PWD ./reproduce.ts
./reproduce.ts => nopAsync ... ok (0ms)

ok | 1 passed | 0 failed (1ms)

$ TEST=nopAsync deno test -A -c ./deno.jsonc --parallel --no-check --no-lock --reload=file:///$PWD ./reproduce.ts
./reproduce.ts => nopAsync ... ok (0ms)

ok | 1 passed | 0 failed (1ms)

$ TEST=nopAsync deno test -A -c ./deno.jsonc --parallel --no-check --no-lock --reload=file:///$PWD ./reproduce.ts
^C
SIGINT The following tests were pending:

nopAsync => ./reproduce.ts:5:10
  • sleepAsync: Promise resolution is still pending but the event loop has already resolved.
$ TEST=sleepAsync deno test -A -c ./deno.jsonc --parallel --no-check --no-lock --reload=file:///$PWD ./reproduce.ts    

ok | 0 passed | 0 failed (1ms)

error: Promise resolution is still pending but the event loop has already resolved.
  • fourtyTwoAsync: Promise resolution is still pending but the event loop has already resolved.
$ TEST=fourtyTwoAsync deno test -A -c ./deno.jsonc --parallel --no-check --no-lock --reload=file:///$PWD ./reproduce.ts

ok | 0 passed | 0 failed (1ms)

error: Promise resolution is still pending but the event loop has already resolved.

expected

above napi async function should not hangs forever, and should be correctly awaited.

@loynoir
Copy link
Author

loynoir commented Apr 6, 2024

Seems like similar bug exists when worker instead of napi.

#21007

@loynoir
Copy link
Author

loynoir commented Apr 6, 2024

Workaround

async function promiseWrapRemotePromiseWithTimeout<T>(p: Promise<T>, delay: number): Promise<T> {
  let timeoutId: undefined | number | NodeJS.Timeout

  return await Promise.race([
    p,
    new Promise<T>((_resolve, reject) => {
      timeoutId = setTimeout(() => {
        reject(new Error())
      }, delay)
    })
  ]).finally(() => {
    if (timeoutId !== undefined) {
      clearTimeout(timeoutId)
    }
  })
}
  case `${fourtyTwoAsync.name}Workaround`:
    Deno.test(fourtyTwoAsync.name, async () => {
      const expected = 42
      const actual = await promiseWrapRemotePromiseWithTimeout(fourtyTwoAsync(100), 3600_000)

      if (actual !== expected) throw new Error()
    })

    break
$ TEST=fourtyTwoAsyncWorkaround deno test -A -c ./deno.jsonc --parallel --no-check --no-lock --reload=file:///$PWD ./reproduce.ts
./reproduce.ts => fourtyTwoAsync ... ok (102ms)

ok | 1 passed | 0 failed (107ms)

@bartlomieju bartlomieju added bug Something isn't working testing related to deno test and coverage node compat node native extension related to the node-api (.node) labels Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working node compat node native extension related to the node-api (.node) testing related to deno test and coverage
Projects
None yet
Development

No branches or pull requests

2 participants