From 1a2928f921f0e0ec2a7ca3fc23aab1542efd589a Mon Sep 17 00:00:00 2001 From: Sacha Froment Date: Mon, 20 Jan 2025 17:11:47 +0100 Subject: [PATCH 1/3] fix(metrics-opentelemetry): isXXX function aren't correct Signed-off-by: Sacha Froment --- packages/metrics-opentelemetry/src/index.ts | 22 +++- .../metrics-opentelemetry/test/index.spec.ts | 111 +++++++++++++++++- 2 files changed, 127 insertions(+), 6 deletions(-) diff --git a/packages/metrics-opentelemetry/src/index.ts b/packages/metrics-opentelemetry/src/index.ts index c0ec0035b0..f7ff837304 100644 --- a/packages/metrics-opentelemetry/src/index.ts +++ b/packages/metrics-opentelemetry/src/index.ts @@ -438,7 +438,7 @@ export function openTelemetryMetrics (init: OpenTelemetryMetricsInit = {}): (com return (components: OpenTelemetryComponents) => new OpenTelemetryMetrics(components, init) } -function isPromise (obj?: any): obj is Promise { +export function isPromise (obj?: any): obj is Promise { return typeof obj?.then === 'function' } @@ -458,8 +458,13 @@ async function wrapPromise (promise: Promise, span: Span, attributes: Trace }) } -function isGenerator (obj?: any): obj is Generator { - return obj?.[Symbol.iterator] != null +export function isGenerator (obj: unknown): obj is Generator { + if (obj == null) return false + const iterator = (obj as { [Symbol.iterator]?: unknown })?.[Symbol.iterator] + if (typeof iterator !== 'function') return false + + const instance = obj as { next?: unknown } + return typeof instance.next === 'function' } function wrapGenerator (gen: Generator, span: Span, attributes: TraceAttributes, options?: TraceGeneratorFunctionOptions): Generator { @@ -502,8 +507,15 @@ function wrapGenerator (gen: Generator, span: Span, attributes: TraceAttributes, return wrapped } -function isAsyncGenerator (obj?: any): obj is AsyncGenerator { - return obj?.[Symbol.asyncIterator] != null +export function isAsyncGenerator (obj: unknown): obj is AsyncGenerator { + if (obj == null) return false + const asyncIterator = (obj as { [Symbol.asyncIterator]?: unknown })?.[ + Symbol.asyncIterator + ] + if (typeof asyncIterator !== 'function') return false + + const instance = obj as { next?: unknown } + return typeof instance.next === 'function' } function wrapAsyncGenerator (gen: AsyncGenerator, span: Span, attributes: TraceAttributes, options?: TraceGeneratorFunctionOptions): AsyncGenerator { diff --git a/packages/metrics-opentelemetry/test/index.spec.ts b/packages/metrics-opentelemetry/test/index.spec.ts index b1011b5ab6..422818d4be 100644 --- a/packages/metrics-opentelemetry/test/index.spec.ts +++ b/packages/metrics-opentelemetry/test/index.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'aegir/chai' -import { openTelemetryMetrics } from '../src/index.js' +import { isPromise, isGenerator, isAsyncGenerator, openTelemetryMetrics } from '../src/index.js' describe('opentelemetry-metrics', () => { it('should wrap a method', async () => { @@ -23,3 +23,112 @@ describe('opentelemetry-metrics', () => { expect(wrapped).to.not.equal(target.wrapped) }) }) + +describe('isPromise', () => { + it('should return true if the value is a promise', () => { + expect(isPromise(Promise.resolve())).to.be.true() + expect(isPromise(new Promise(() => {}))).to.be.true() + expect(isPromise(Promise.reject(new Error('test')))).to.be.true() + }) + + it('should return false if the value is not a promise', () => { + expect(isPromise(1)).to.be.false() + expect(isPromise('string')).to.be.false() + expect(isPromise({})).to.be.false() + expect(isPromise([])).to.be.false() + expect(isPromise(null)).to.be.false() + expect(isPromise(undefined)).to.be.false() + expect(isPromise(() => {})).to.be.false() + expect(isPromise(async () => {})).to.be.false() + expect( + isPromise(function * () { + yield 1 + }) + ).to.be.false() + expect( + isPromise(async function * () { + yield 1 + }) + ).to.be.false() + // biome-ignore lint/suspicious/noThenProperty: for testing purposes + expect(isPromise({ then: 1 })).to.be.false() + }) +}) + +describe('isGenerator', () => { + it('should return true if the value is a generator', () => { + function * gen (): Generator { + yield 1 + } + const generator = gen() + expect(isGenerator(generator)).to.be.true() + + const genObj = (function * () { + yield 1 + })() + expect(isGenerator(genObj)).to.be.true() + }) + + it('should return false if the value is not a generator', () => { + expect(isGenerator(1)).to.be.false() + expect(isGenerator('string')).to.be.false() + expect(isGenerator({})).to.be.false() + expect(isGenerator([])).to.be.false() + expect(isGenerator(null)).to.be.false() + expect(isGenerator(undefined)).to.be.false() + expect(isGenerator(() => {})).to.be.false() + expect(isGenerator(async () => {})).to.be.false() + expect( + isGenerator(function * () { + yield 1 + }) + ).to.be.false() // generator function, not generator + expect( + isGenerator(async function * () { + yield 1 + }) + ).to.be.false() + expect(isGenerator(Promise.resolve())).to.be.false() + expect(isGenerator({ next: () => {} })).to.be.false() + expect(isGenerator({ [Symbol.iterator]: () => {} })).to.be.false() + }) +}) + +describe('isAsyncGenerator', () => { + it('should return true if the value is an async generator', () => { + async function * asyncGen (): AsyncGenerator { + yield 1 + } + const asyncGenerator = asyncGen() + expect(isAsyncGenerator(asyncGenerator)).to.be.true() + + const asyncGenObj = (async function * () { + yield 1 + })() + expect(isAsyncGenerator(asyncGenObj)).to.be.true() + }) + + it('should return false if the value is not an async generator', () => { + expect(isAsyncGenerator(1)).to.be.false() + expect(isAsyncGenerator('string')).to.be.false() + expect(isAsyncGenerator({})).to.be.false() + expect(isAsyncGenerator([])).to.be.false() + expect(isAsyncGenerator(null)).to.be.false() + expect(isAsyncGenerator(undefined)).to.be.false() + expect(isAsyncGenerator(() => {})).to.be.false() + expect(isAsyncGenerator(async () => {})).to.be.false() + expect( + isAsyncGenerator(function * () { + yield 1 + }) + ).to.be.false() + expect( + isAsyncGenerator(async function * () { + yield 1 + }) + ).to.be.false() // async generator function, not generator + expect(isAsyncGenerator(Promise.resolve())).to.be.false() + expect(isAsyncGenerator({ next: async () => {} })).to.be.false() + expect(isAsyncGenerator({ [Symbol.asyncIterator]: () => {} })).to.be.false() + }) +}) From 6c6362a820566ee2282631faa404aa8d8eebdadc Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 27 Jan 2025 15:18:01 +0100 Subject: [PATCH 2/3] chore: use utils exports --- packages/metrics-opentelemetry/package.json | 1 + packages/metrics-opentelemetry/src/index.ts | 27 +---- .../metrics-opentelemetry/test/index.spec.ts | 111 +----------------- 3 files changed, 5 insertions(+), 134 deletions(-) diff --git a/packages/metrics-opentelemetry/package.json b/packages/metrics-opentelemetry/package.json index 54e314b534..0822f104fa 100644 --- a/packages/metrics-opentelemetry/package.json +++ b/packages/metrics-opentelemetry/package.json @@ -48,6 +48,7 @@ }, "dependencies": { "@libp2p/interface": "^2.4.0", + "@libp2p/utils": "^6.3.1", "@opentelemetry/api": "^1.9.0", "it-foreach": "^2.1.1", "it-stream-types": "^2.0.2" diff --git a/packages/metrics-opentelemetry/src/index.ts b/packages/metrics-opentelemetry/src/index.ts index f7ff837304..ebc8f3014e 100644 --- a/packages/metrics-opentelemetry/src/index.ts +++ b/packages/metrics-opentelemetry/src/index.ts @@ -34,6 +34,9 @@ */ import { InvalidParametersError, serviceCapabilities } from '@libp2p/interface' +import { isAsyncGenerator } from '@libp2p/utils/is-async-generator' +import { isGenerator } from '@libp2p/utils/is-generator' +import { isPromise } from '@libp2p/utils/is-promise' import { trace, metrics, context, SpanStatusCode } from '@opentelemetry/api' import each from 'it-foreach' import { OpenTelemetryCounterGroup } from './counter-group.js' @@ -438,10 +441,6 @@ export function openTelemetryMetrics (init: OpenTelemetryMetricsInit = {}): (com return (components: OpenTelemetryComponents) => new OpenTelemetryMetrics(components, init) } -export function isPromise (obj?: any): obj is Promise { - return typeof obj?.then === 'function' -} - async function wrapPromise (promise: Promise, span: Span, attributes: TraceAttributes, options?: TraceFunctionOptions): Promise { return promise .then(res => { @@ -458,15 +457,6 @@ async function wrapPromise (promise: Promise, span: Span, attributes: Trace }) } -export function isGenerator (obj: unknown): obj is Generator { - if (obj == null) return false - const iterator = (obj as { [Symbol.iterator]?: unknown })?.[Symbol.iterator] - if (typeof iterator !== 'function') return false - - const instance = obj as { next?: unknown } - return typeof instance.next === 'function' -} - function wrapGenerator (gen: Generator, span: Span, attributes: TraceAttributes, options?: TraceGeneratorFunctionOptions): Generator { const iter = gen[Symbol.iterator]() let index = 0 @@ -507,17 +497,6 @@ function wrapGenerator (gen: Generator, span: Span, attributes: TraceAttributes, return wrapped } -export function isAsyncGenerator (obj: unknown): obj is AsyncGenerator { - if (obj == null) return false - const asyncIterator = (obj as { [Symbol.asyncIterator]?: unknown })?.[ - Symbol.asyncIterator - ] - if (typeof asyncIterator !== 'function') return false - - const instance = obj as { next?: unknown } - return typeof instance.next === 'function' -} - function wrapAsyncGenerator (gen: AsyncGenerator, span: Span, attributes: TraceAttributes, options?: TraceGeneratorFunctionOptions): AsyncGenerator { const iter = gen[Symbol.asyncIterator]() let index = 0 diff --git a/packages/metrics-opentelemetry/test/index.spec.ts b/packages/metrics-opentelemetry/test/index.spec.ts index 422818d4be..b1011b5ab6 100644 --- a/packages/metrics-opentelemetry/test/index.spec.ts +++ b/packages/metrics-opentelemetry/test/index.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'aegir/chai' -import { isPromise, isGenerator, isAsyncGenerator, openTelemetryMetrics } from '../src/index.js' +import { openTelemetryMetrics } from '../src/index.js' describe('opentelemetry-metrics', () => { it('should wrap a method', async () => { @@ -23,112 +23,3 @@ describe('opentelemetry-metrics', () => { expect(wrapped).to.not.equal(target.wrapped) }) }) - -describe('isPromise', () => { - it('should return true if the value is a promise', () => { - expect(isPromise(Promise.resolve())).to.be.true() - expect(isPromise(new Promise(() => {}))).to.be.true() - expect(isPromise(Promise.reject(new Error('test')))).to.be.true() - }) - - it('should return false if the value is not a promise', () => { - expect(isPromise(1)).to.be.false() - expect(isPromise('string')).to.be.false() - expect(isPromise({})).to.be.false() - expect(isPromise([])).to.be.false() - expect(isPromise(null)).to.be.false() - expect(isPromise(undefined)).to.be.false() - expect(isPromise(() => {})).to.be.false() - expect(isPromise(async () => {})).to.be.false() - expect( - isPromise(function * () { - yield 1 - }) - ).to.be.false() - expect( - isPromise(async function * () { - yield 1 - }) - ).to.be.false() - // biome-ignore lint/suspicious/noThenProperty: for testing purposes - expect(isPromise({ then: 1 })).to.be.false() - }) -}) - -describe('isGenerator', () => { - it('should return true if the value is a generator', () => { - function * gen (): Generator { - yield 1 - } - const generator = gen() - expect(isGenerator(generator)).to.be.true() - - const genObj = (function * () { - yield 1 - })() - expect(isGenerator(genObj)).to.be.true() - }) - - it('should return false if the value is not a generator', () => { - expect(isGenerator(1)).to.be.false() - expect(isGenerator('string')).to.be.false() - expect(isGenerator({})).to.be.false() - expect(isGenerator([])).to.be.false() - expect(isGenerator(null)).to.be.false() - expect(isGenerator(undefined)).to.be.false() - expect(isGenerator(() => {})).to.be.false() - expect(isGenerator(async () => {})).to.be.false() - expect( - isGenerator(function * () { - yield 1 - }) - ).to.be.false() // generator function, not generator - expect( - isGenerator(async function * () { - yield 1 - }) - ).to.be.false() - expect(isGenerator(Promise.resolve())).to.be.false() - expect(isGenerator({ next: () => {} })).to.be.false() - expect(isGenerator({ [Symbol.iterator]: () => {} })).to.be.false() - }) -}) - -describe('isAsyncGenerator', () => { - it('should return true if the value is an async generator', () => { - async function * asyncGen (): AsyncGenerator { - yield 1 - } - const asyncGenerator = asyncGen() - expect(isAsyncGenerator(asyncGenerator)).to.be.true() - - const asyncGenObj = (async function * () { - yield 1 - })() - expect(isAsyncGenerator(asyncGenObj)).to.be.true() - }) - - it('should return false if the value is not an async generator', () => { - expect(isAsyncGenerator(1)).to.be.false() - expect(isAsyncGenerator('string')).to.be.false() - expect(isAsyncGenerator({})).to.be.false() - expect(isAsyncGenerator([])).to.be.false() - expect(isAsyncGenerator(null)).to.be.false() - expect(isAsyncGenerator(undefined)).to.be.false() - expect(isAsyncGenerator(() => {})).to.be.false() - expect(isAsyncGenerator(async () => {})).to.be.false() - expect( - isAsyncGenerator(function * () { - yield 1 - }) - ).to.be.false() - expect( - isAsyncGenerator(async function * () { - yield 1 - }) - ).to.be.false() // async generator function, not generator - expect(isAsyncGenerator(Promise.resolve())).to.be.false() - expect(isAsyncGenerator({ next: async () => {} })).to.be.false() - expect(isAsyncGenerator({ [Symbol.asyncIterator]: () => {} })).to.be.false() - }) -}) From d68bfc06ebf42bd1d76b2c637aae2594535899d3 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 27 Jan 2025 15:23:36 +0100 Subject: [PATCH 3/3] chore: legibility --- packages/utils/src/is-async-generator.ts | 10 ++++++++-- packages/utils/src/is-generator.ts | 11 +++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/utils/src/is-async-generator.ts b/packages/utils/src/is-async-generator.ts index 73a7a20b6b..57a4a9ed1c 100644 --- a/packages/utils/src/is-async-generator.ts +++ b/packages/utils/src/is-async-generator.ts @@ -1,9 +1,15 @@ export function isAsyncGenerator (obj: unknown): obj is AsyncGenerator { - if (obj == null) return false + if (obj == null) { + return false + } + const asyncIterator = (obj as { [Symbol.asyncIterator]?: unknown })?.[ Symbol.asyncIterator ] - if (typeof asyncIterator !== 'function') return false + + if (typeof asyncIterator !== 'function') { + return false + } const instance = obj as { next?: unknown } return typeof instance.next === 'function' diff --git a/packages/utils/src/is-generator.ts b/packages/utils/src/is-generator.ts index ee5825412d..6307cd4457 100644 --- a/packages/utils/src/is-generator.ts +++ b/packages/utils/src/is-generator.ts @@ -1,8 +1,15 @@ export function isGenerator (obj: unknown): obj is Generator { - if (obj == null) return false + if (obj == null) { + return false + } + const iterator = (obj as { [Symbol.iterator]?: unknown })?.[Symbol.iterator] - if (typeof iterator !== 'function') return false + + if (typeof iterator !== 'function') { + return false + } const instance = obj as { next?: unknown } + return typeof instance.next === 'function' }