adonisjs / adonisjs/otel

record helper / span decorators break methods returning objects that are both Promise and AsyncIterable

Open
#8 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
15
Forks
2
PR merge metrics
No merged PRs in 30d

Description

Package version

1.2.3

Describe the bug
Bug

This is a niche issue, but I have some classes that wrap Stripe SDK methods which implement auto-agination. They do this by returning an ApiListPromise, a class that extends Promise<ApiList<T>> AND implements AsyncIterable<T>

Wrapping these in record() (or adding a @span or @spanAll decorator) breaks these, as record checks instanceof Promise and calls .then() on the return value

Can understand if this is one not worth fixing as it's so niche, but happy to work on a PR otherwise

In my app code the error looks like so, where StripeService.listSubscriptions has @span

ℹ TypeError:  this.stripe.listSubscriptions(...) is not a function or its return value is not  async iterable
 ⁃ at StripeSyncProvider.hydrateSubscriptions (providers/stripe_sync_provider.ts:284:47)

 ❯ 284 ┃      for await (const stripeSub of this.stripeService.listSubscriptions()) {
 ⁃ at <anonymous> (providers/stripe_sync_provider.ts:252:18)
Failing test-case
test('record preserves AsyncIterable interface on Promise+AsyncIterable return values', async ({ assert }) => {
  const items = [1, 2, 3]
  const asyncIterablePromise = Object.assign(Promise.resolve(items), {
    async *[Symbol.asyncIterator]() {
      for (const item of items) {
        yield item
      }
    },
  })

  const result = record('iterable-op', () => {
    return asyncIterablePromise
  })

  assert.isTrue(
    Symbol.asyncIterator in result,
    'record() should preserve AsyncIterable interface'
  )

  const collected: number[] = []
  for await (const item of result as AsyncIterable<number>) {
    collected.push(item)
  }
  assert.deepEqual(collected, [1, 2, 3])
})

Which results in these 2 failures (the latter if the first is disabled)

ℹ AssertionError: record() should preserve AsyncIterable interface: expected false to be true
 ❯ 158 ┃      assert.isTrue(
   159 ┃        Symbol.asyncIterator in result,
   160 ┃        'record() should preserve AsyncIterable interface'
   161 ┃      )
ℹ TypeError: result is not async iterable
   163 ┃      const collected: number[] = []
 ❯ 164 ┃      for await (const item of result as AsyncIterable<number>) {
Reproduction repo

No response

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the record() helper and the @span/@spanAll decorators, then run the supplied test, “record preserves AsyncIterable interface on Promise+AsyncIterable return values.” The change is done when record() preserves Symbol.asyncIterator for values that are both Promise and AsyncIterable, and the test passes while collecting [1, 2, 3].

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
observability
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.