From 82d00accde39327f842a8481688623755bd4031a Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 12:39:23 -0400 Subject: [PATCH 01/29] feat(diagnostics): add enableDiagnosticsChannel and channel registry Introduces src/diagnostics.ts which exposes a single public function, enableDiagnosticsChannel(dc), that APMs use to register a node:diagnostics_channel-compatible module with graphql-js. Channel names (graphql:parse, graphql:validate, graphql:execute, graphql:resolve, graphql:subscribe) are owned by graphql-js so multiple APM subscribers converge on the same cached TracingChannel instances. Structural MinimalChannel / MinimalTracingChannel / MinimalDiagnosticsChannel types describe the subset of the Node API graphql-js needs; no dependency on @types/node is introduced, and no runtime-specific import is added to the core. Subsequent commits wire emission sites into parse, validate, execute, subscribe, and the resolver path. --- src/__tests__/diagnostics-test.ts | 111 ++++++++++++++++++++++++++ src/diagnostics.ts | 127 ++++++++++++++++++++++++++++++ src/index.ts | 11 +++ 3 files changed, 249 insertions(+) create mode 100644 src/__tests__/diagnostics-test.ts create mode 100644 src/diagnostics.ts diff --git a/src/__tests__/diagnostics-test.ts b/src/__tests__/diagnostics-test.ts new file mode 100644 index 0000000000..e620c86fd7 --- /dev/null +++ b/src/__tests__/diagnostics-test.ts @@ -0,0 +1,111 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { invariant } from '../jsutils/invariant.js'; + +import type { + MinimalDiagnosticsChannel, + MinimalTracingChannel, +} from '../diagnostics.js'; +import { enableDiagnosticsChannel, getChannels } from '../diagnostics.js'; + +function fakeTracingChannel(name: string): MinimalTracingChannel { + const noop: MinimalTracingChannel['start'] = { + hasSubscribers: false, + publish: () => { + /* noop */ + }, + runStores: ( + _ctx: unknown, + fn: (this: unknown, ...args: Array) => T, + ): T => fn(), + }; + const channel: MinimalTracingChannel & { _name: string } = { + _name: name, + hasSubscribers: false, + start: noop, + end: noop, + asyncStart: noop, + asyncEnd: noop, + error: noop, + traceSync: (fn: (...args: Array) => T): T => fn(), + tracePromise: ( + fn: (...args: Array) => Promise, + ): Promise => fn(), + }; + return channel; +} + +function fakeDc(): MinimalDiagnosticsChannel & { + created: Array; +} { + const created: Array = []; + const cache = new Map(); + return { + created, + tracingChannel(name: string) { + let existing = cache.get(name); + if (existing === undefined) { + created.push(name); + existing = fakeTracingChannel(name); + cache.set(name, existing); + } + return existing; + }, + }; +} + +describe('diagnostics', () => { + it('registers the five graphql tracing channels', () => { + const dc = fakeDc(); + enableDiagnosticsChannel(dc); + + expect(dc.created).to.deep.equal([ + 'graphql:execute', + 'graphql:parse', + 'graphql:validate', + 'graphql:resolve', + 'graphql:subscribe', + ]); + + const channels = getChannels(); + invariant(channels !== undefined); + expect(channels.execute).to.not.equal(undefined); + expect(channels.parse).to.not.equal(undefined); + expect(channels.validate).to.not.equal(undefined); + expect(channels.resolve).to.not.equal(undefined); + expect(channels.subscribe).to.not.equal(undefined); + }); + + it('re-registration with the same module preserves channel identity', () => { + const dc = fakeDc(); + enableDiagnosticsChannel(dc); + const first = getChannels(); + invariant(first !== undefined); + + enableDiagnosticsChannel(dc); + const second = getChannels(); + invariant(second !== undefined); + + expect(second.execute).to.equal(first.execute); + expect(second.parse).to.equal(first.parse); + expect(second.validate).to.equal(first.validate); + expect(second.resolve).to.equal(first.resolve); + expect(second.subscribe).to.equal(first.subscribe); + }); + + it('re-registration with a different module replaces stored references', () => { + const dc1 = fakeDc(); + const dc2 = fakeDc(); + + enableDiagnosticsChannel(dc1); + const first = getChannels(); + invariant(first !== undefined); + + enableDiagnosticsChannel(dc2); + const second = getChannels(); + invariant(second !== undefined); + + expect(second.execute).to.not.equal(first.execute); + }); +}); diff --git a/src/diagnostics.ts b/src/diagnostics.ts new file mode 100644 index 0000000000..2e611312bd --- /dev/null +++ b/src/diagnostics.ts @@ -0,0 +1,127 @@ +/** + * TracingChannel integration. + * + * graphql-js exposes a set of named tracing channels that APM tools can + * subscribe to in order to observe parse, validate, execute, subscribe, and + * resolver lifecycle events. To preserve the isomorphic invariant of the + * core (no runtime-specific imports in `src/`), graphql-js does not import + * `node:diagnostics_channel` itself. Instead, APMs (or runtime-specific + * adapters) hand in a module satisfying `MinimalDiagnosticsChannel` via + * `enableDiagnosticsChannel`. + * + * Channel names are owned by graphql-js so multiple APMs converge on the + * same `TracingChannel` instances and all subscribers coexist. + */ + +/** + * Structural subset of `DiagnosticsChannel` sufficient for publishing and + * subscriber gating. `node:diagnostics_channel`'s `Channel` satisfies this. + */ +export interface MinimalChannel { + readonly hasSubscribers: boolean; + publish: (message: unknown) => void; + runStores: ( + context: ContextType, + fn: (this: ContextType, ...args: Array) => T, + thisArg?: unknown, + ...args: Array + ) => T; +} + +/** + * Structural subset of Node's `TracingChannel`. The `node:diagnostics_channel` + * `TracingChannel` satisfies this by duck typing, so graphql-js does not need + * a dependency on `@types/node` or on the runtime itself. + */ +export interface MinimalTracingChannel { + readonly hasSubscribers: boolean; + readonly start: MinimalChannel; + readonly end: MinimalChannel; + readonly asyncStart: MinimalChannel; + readonly asyncEnd: MinimalChannel; + readonly error: MinimalChannel; + + traceSync: ( + fn: (...args: Array) => T, + ctx: object, + thisArg?: unknown, + ...args: Array + ) => T; + + tracePromise: ( + fn: (...args: Array) => Promise, + ctx: object, + thisArg?: unknown, + ...args: Array + ) => Promise; +} + +/** + * Structural subset of `node:diagnostics_channel` covering just what + * graphql-js needs at registration time. + */ +export interface MinimalDiagnosticsChannel { + tracingChannel: (name: string) => MinimalTracingChannel; +} + +/** + * The collection of tracing channels graphql-js emits on. APMs subscribe to + * these by name on their own `node:diagnostics_channel` import; both paths + * land on the same channel instance because `tracingChannel(name)` is cached + * by name. + */ +export interface GraphQLChannels { + execute: MinimalTracingChannel; + parse: MinimalTracingChannel; + validate: MinimalTracingChannel; + resolve: MinimalTracingChannel; + subscribe: MinimalTracingChannel; +} + +let channels: GraphQLChannels | undefined; + +/** + * Internal accessor used at emission sites. Returns `undefined` when no + * `diagnostics_channel` module has been registered, allowing emission sites + * to short-circuit on a single property access. + * + * @internal + */ +export function getChannels(): GraphQLChannels | undefined { + return channels; +} + +/** + * Register a `node:diagnostics_channel`-compatible module with graphql-js. + * + * After calling this, graphql-js will publish lifecycle events on the + * following tracing channels whenever subscribers are present: + * + * - `graphql:parse` + * - `graphql:validate` + * - `graphql:execute` + * - `graphql:subscribe` + * - `graphql:resolve` + * + * Calling this repeatedly is safe: subsequent calls replace the stored + * channel references, but since `tracingChannel(name)` is cached by name, + * the channel identities remain stable across registrations from the same + * underlying module. + * + * @example + * ```ts + * import dc from 'node:diagnostics_channel'; + * import { enableDiagnosticsChannel } from 'graphql'; + * + * enableDiagnosticsChannel(dc); + * ``` + */ +export function enableDiagnosticsChannel(dc: MinimalDiagnosticsChannel): void { + channels = { + execute: dc.tracingChannel('graphql:execute'), + parse: dc.tracingChannel('graphql:parse'), + validate: dc.tracingChannel('graphql:validate'), + resolve: dc.tracingChannel('graphql:resolve'), + subscribe: dc.tracingChannel('graphql:subscribe'), + }; +} diff --git a/src/index.ts b/src/index.ts index f19286f4bb..f714d2ea60 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,6 +32,17 @@ export { version, versionInfo } from './version.js'; // Enable development mode for additional checks. export { enableDevMode, isDevModeEnabled } from './devMode.js'; +// Register a `node:diagnostics_channel`-compatible module to enable +// tracing channel emission from parse, validate, execute, subscribe, +// and resolver lifecycles. +export { enableDiagnosticsChannel } from './diagnostics.js'; +export type { + MinimalChannel, + MinimalTracingChannel, + MinimalDiagnosticsChannel, + GraphQLChannels, +} from './diagnostics.js'; + // The primary entry point into fulfilling a GraphQL request. export type { GraphQLArgs } from './graphql.js'; export { graphql, graphqlSync } from './graphql.js'; From 1d469bb51cd0cc1c696d4d71a578f44a3ebb3fd3 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 12:57:01 -0400 Subject: [PATCH 02/29] feat(language): publish on graphql:parse tracing channel Wraps parse() with the graphql:parse channel when any subscribers are attached. The published context carries the Source (or string) handed to parse() so APM tools can record the GraphQL source on their span. Adds two shared helpers on src/diagnostics.ts used by every subsequent emission site: maybeTraceSync(name, ctxFactory, fn) maybeTracePromise(name, ctxFactory, fn) They share a shouldTrace gate that uses hasSubscribers !== false so runtimes without an aggregated hasSubscribers getter (notably Node 18, see nodejs/node#54470) still publish; Node's traceSync/tracePromise gate each sub-channel internally. Context construction is lazy through the factory so the no-subscriber path allocates nothing beyond the closure. --- src/diagnostics.ts | 58 ++++- .../__tests__/parser-diagnostics-test.ts | 214 ++++++++++++++++++ src/language/parser.ts | 22 +- 3 files changed, 286 insertions(+), 8 deletions(-) create mode 100644 src/language/__tests__/parser-diagnostics-test.ts diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 2e611312bd..b37c3f4a8e 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -20,7 +20,7 @@ export interface MinimalChannel { readonly hasSubscribers: boolean; publish: (message: unknown) => void; - runStores: ( + runStores: ( context: ContextType, fn: (this: ContextType, ...args: Array) => T, thisArg?: unknown, @@ -125,3 +125,59 @@ export function enableDiagnosticsChannel(dc: MinimalDiagnosticsChannel): void { subscribe: dc.tracingChannel('graphql:subscribe'), }; } + +/** + * Gate for emission sites. Returns `true` when the named channel exists and + * publishing should proceed. + * + * Uses `!== false` rather than a truthy check so runtimes which do not + * implement the aggregated `hasSubscribers` getter on `TracingChannel` still + * publish. Notably Node 18 (nodejs/node#54470), where the aggregated getter + * returns `undefined` while sub-channels behave correctly. + * + * @internal + */ +function shouldTrace( + channel: MinimalTracingChannel | undefined, +): channel is MinimalTracingChannel { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare + return channel !== undefined && channel.hasSubscribers !== false; +} + +/** + * Publish a synchronous operation through the named graphql tracing channel, + * short-circuiting to `fn()` when the channel isn't registered or nothing is + * listening. + * + * @internal + */ +export function maybeTraceSync( + name: keyof GraphQLChannels, + ctxFactory: () => object, + fn: () => T, +): T { + const channel = getChannels()?.[name]; + if (!shouldTrace(channel)) { + return fn(); + } + return channel.traceSync(fn, ctxFactory()); +} + +/** + * Publish a promise-returning operation through the named graphql tracing + * channel, short-circuiting to `fn()` when the channel isn't registered or + * nothing is listening. + * + * @internal + */ +export function maybeTracePromise( + name: keyof GraphQLChannels, + ctxFactory: () => object, + fn: () => Promise, +): Promise { + const channel = getChannels()?.[name]; + if (!shouldTrace(channel)) { + return fn(); + } + return channel.tracePromise(fn, ctxFactory()); +} diff --git a/src/language/__tests__/parser-diagnostics-test.ts b/src/language/__tests__/parser-diagnostics-test.ts new file mode 100644 index 0000000000..ac16f46bce --- /dev/null +++ b/src/language/__tests__/parser-diagnostics-test.ts @@ -0,0 +1,214 @@ +import { expect } from 'chai'; +import { afterEach, beforeEach, describe, it } from 'mocha'; + +import type { + MinimalChannel, + MinimalDiagnosticsChannel, + MinimalTracingChannel, +} from '../../diagnostics.js'; +import { enableDiagnosticsChannel } from '../../diagnostics.js'; + +import { parse } from '../parser.js'; + +type Listener = (message: unknown) => void; + +class FakeChannel implements MinimalChannel { + listeners: Array = []; + get hasSubscribers(): boolean { + return this.listeners.length > 0; + } + + publish(message: unknown): void { + for (const l of this.listeners) { + l(message); + } + } + + runStores( + _ctx: ContextType, + fn: (this: ContextType, ...args: Array) => T, + thisArg?: unknown, + ...args: Array + ): T { + return fn.apply(thisArg as ContextType, args); + } + + subscribe(listener: Listener): void { + this.listeners.push(listener); + } + + unsubscribe(listener: Listener): void { + const idx = this.listeners.indexOf(listener); + if (idx >= 0) { + this.listeners.splice(idx, 1); + } + } +} + +class FakeTracingChannel implements MinimalTracingChannel { + start = new FakeChannel(); + end = new FakeChannel(); + asyncStart = new FakeChannel(); + asyncEnd = new FakeChannel(); + error = new FakeChannel(); + + get hasSubscribers(): boolean { + return ( + this.start.hasSubscribers || + this.end.hasSubscribers || + this.asyncStart.hasSubscribers || + this.asyncEnd.hasSubscribers || + this.error.hasSubscribers + ); + } + + traceSync( + fn: (...args: Array) => T, + ctx: object, + thisArg?: unknown, + ...args: Array + ): T { + this.start.publish(ctx); + try { + return this.end.runStores(ctx, fn, thisArg, ...args); + } catch (err) { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + throw err; + } finally { + this.end.publish(ctx); + } + } + + tracePromise( + fn: (...args: Array) => Promise, + ctx: object, + thisArg?: unknown, + ...args: Array + ): Promise { + this.start.publish(ctx); + let promise: Promise; + try { + promise = this.end.runStores(ctx, fn, thisArg, ...args); + } catch (err) { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + this.end.publish(ctx); + throw err; + } + this.end.publish(ctx); + return promise.then( + (result) => { + (ctx as { result: unknown }).result = result; + this.asyncStart.publish(ctx); + this.asyncEnd.publish(ctx); + return result; + }, + (err: unknown) => { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + this.asyncStart.publish(ctx); + this.asyncEnd.publish(ctx); + throw err; + }, + ); + } +} + +class FakeDc implements MinimalDiagnosticsChannel { + private cache = new Map(); + + tracingChannel(name: string): FakeTracingChannel { + let existing = this.cache.get(name); + if (existing === undefined) { + existing = new FakeTracingChannel(); + this.cache.set(name, existing); + } + return existing; + } +} + +const fakeDc = new FakeDc(); +const parseChannel = fakeDc.tracingChannel('graphql:parse'); + +interface Event { + kind: 'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error'; + source: unknown; + error?: unknown; +} + +function collectEvents(): { events: Array; unsubscribe: () => void } { + const events: Array = []; + const startL: Listener = (m) => + events.push({ kind: 'start', source: (m as { source: unknown }).source }); + const endL: Listener = (m) => + events.push({ kind: 'end', source: (m as { source: unknown }).source }); + const asyncStartL: Listener = (m) => + events.push({ + kind: 'asyncStart', + source: (m as { source: unknown }).source, + }); + const asyncEndL: Listener = (m) => + events.push({ + kind: 'asyncEnd', + source: (m as { source: unknown }).source, + }); + const errorL: Listener = (m) => { + const msg = m as { source: unknown; error: unknown }; + events.push({ kind: 'error', source: msg.source, error: msg.error }); + }; + parseChannel.start.subscribe(startL); + parseChannel.end.subscribe(endL); + parseChannel.asyncStart.subscribe(asyncStartL); + parseChannel.asyncEnd.subscribe(asyncEndL); + parseChannel.error.subscribe(errorL); + return { + events, + unsubscribe() { + parseChannel.start.unsubscribe(startL); + parseChannel.end.unsubscribe(endL); + parseChannel.asyncStart.unsubscribe(asyncStartL); + parseChannel.asyncEnd.unsubscribe(asyncEndL); + parseChannel.error.unsubscribe(errorL); + }, + }; +} + +describe('parse diagnostics channel', () => { + let active: ReturnType | undefined; + + beforeEach(() => { + enableDiagnosticsChannel(fakeDc); + }); + + afterEach(() => { + active?.unsubscribe(); + active = undefined; + }); + + it('emits start and end around a successful parse', () => { + active = collectEvents(); + + const doc = parse('{ field }'); + + expect(doc.kind).to.equal('Document'); + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + expect(active.events[0].source).to.equal('{ field }'); + expect(active.events[1].source).to.equal('{ field }'); + }); + + it('emits start, error, and end when the parser throws', () => { + active = collectEvents(); + + expect(() => parse('{ ')).to.throw(); + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal(['start', 'error', 'end']); + expect(active.events[1].error).to.be.instanceOf(Error); + }); + + it('does nothing when no subscribers are attached', () => { + const doc = parse('{ field }'); + expect(doc.kind).to.equal('Document'); + }); +}); diff --git a/src/language/parser.ts b/src/language/parser.ts index d9e6dd8ba2..ba62a9ccb0 100644 --- a/src/language/parser.ts +++ b/src/language/parser.ts @@ -3,6 +3,8 @@ import type { Maybe } from '../jsutils/Maybe.js'; import type { GraphQLError } from '../error/GraphQLError.js'; import { syntaxError } from '../error/syntaxError.js'; +import { maybeTraceSync } from '../diagnostics.js'; + import type { ArgumentCoordinateNode, ArgumentNode, @@ -132,13 +134,19 @@ export function parse( source: string | Source, options?: ParseOptions, ): DocumentNode { - const parser = new Parser(source, options); - const document = parser.parseDocument(); - Object.defineProperty(document, 'tokenCount', { - enumerable: false, - value: parser.tokenCount, - }); - return document; + return maybeTraceSync( + 'parse', + () => ({ source }), + () => { + const parser = new Parser(source, options); + const document = parser.parseDocument(); + Object.defineProperty(document, 'tokenCount', { + enumerable: false, + value: parser.tokenCount, + }); + return document; + }, + ); } /** From 23124e50480b46e64d60c569c8cc2e80c6f3c2b7 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 13:08:48 -0400 Subject: [PATCH 03/29] test(integration): exercise graphql tracing channels on real node:diagnostics_channel Adds an integrationTests/diagnostics/ project that installs the built graphql package, registers node:diagnostics_channel, and asserts that subscribers to graphql:parse receive the expected start/end/error lifecycle events. Complements the unit tests (which use a FakeDc) with coverage against a real TracingChannel. The in-tree eslint rules forbid node: imports and flag TracingChannel as experimental in src/; integrationTests/* overrides already allow the node: import, and a file-scope disable handles the experimental warning. The integration project pins engines to >=22 to match where graphql-js v17 is actively tested. This file grows as later channels (validate, execute, subscribe, resolve) are wired up. --- integrationTests/diagnostics/package.json | 14 +++++ integrationTests/diagnostics/test.js | 73 +++++++++++++++++++++++ resources/integration-test.ts | 3 + 3 files changed, 90 insertions(+) create mode 100644 integrationTests/diagnostics/package.json create mode 100644 integrationTests/diagnostics/test.js diff --git a/integrationTests/diagnostics/package.json b/integrationTests/diagnostics/package.json new file mode 100644 index 0000000000..0face40966 --- /dev/null +++ b/integrationTests/diagnostics/package.json @@ -0,0 +1,14 @@ +{ + "description": "graphql-js tracing channels should publish on node:diagnostics_channel", + "private": true, + "type": "module", + "engines": { + "node": ">=22.0.0" + }, + "scripts": { + "test": "node test.js" + }, + "dependencies": { + "graphql": "file:../graphql.tgz" + } +} diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js new file mode 100644 index 0000000000..646ceb7046 --- /dev/null +++ b/integrationTests/diagnostics/test.js @@ -0,0 +1,73 @@ +// TracingChannel is marked experimental in Node's docs but is shipped on +// every runtime graphql-js supports. This test exercises it directly. +/* eslint-disable n/no-unsupported-features/node-builtins */ + +import assert from 'node:assert/strict'; +import dc from 'node:diagnostics_channel'; + +import { enableDiagnosticsChannel, parse } from 'graphql'; + +enableDiagnosticsChannel(dc); + +// graphql:parse - synchronous +{ + const events = []; + const handler = { + start: (msg) => events.push({ kind: 'start', source: msg.source }), + end: (msg) => events.push({ kind: 'end', source: msg.source }), + asyncStart: (msg) => + events.push({ kind: 'asyncStart', source: msg.source }), + asyncEnd: (msg) => events.push({ kind: 'asyncEnd', source: msg.source }), + error: (msg) => + events.push({ kind: 'error', source: msg.source, error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:parse'); + channel.subscribe(handler); + + try { + const doc = parse('{ field }'); + assert.equal(doc.kind, 'Document'); + assert.deepEqual( + events.map((e) => e.kind), + ['start', 'end'], + ); + assert.equal(events[0].source, '{ field }'); + assert.equal(events[1].source, '{ field }'); + } finally { + channel.unsubscribe(handler); + } +} + +// graphql:parse - error path fires start, error, end (traceSync finally-emits end) +{ + const events = []; + const handler = { + start: (msg) => events.push({ kind: 'start', source: msg.source }), + end: (msg) => events.push({ kind: 'end', source: msg.source }), + error: (msg) => + events.push({ kind: 'error', source: msg.source, error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:parse'); + channel.subscribe(handler); + + try { + assert.throws(() => parse('{ ')); + assert.deepEqual( + events.map((e) => e.kind), + ['start', 'error', 'end'], + ); + assert.ok(events[1].error instanceof Error); + } finally { + channel.unsubscribe(handler); + } +} + +// No-op when nothing is subscribed - parse still succeeds. +{ + const doc = parse('{ field }'); + assert.equal(doc.kind, 'Document'); +} + +console.log('diagnostics integration test passed'); diff --git a/resources/integration-test.ts b/resources/integration-test.ts index 1eb240d5d5..0e584726b0 100644 --- a/resources/integration-test.ts +++ b/resources/integration-test.ts @@ -55,6 +55,9 @@ describe('Integration Tests', () => { testOnNodeProject('node'); testOnNodeProject('webpack'); + // Tracing channel tests + testOnNodeProject('diagnostics'); + // Conditional export tests testOnNodeProject('conditions'); From 04a3054078c6d428a0c1b87de9ae7e985f4ab5ce Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 13:15:20 -0400 Subject: [PATCH 04/29] feat(validation): publish on graphql:validate tracing channel Wraps the public validate() entry point with maybeTraceSync. The context carries both the schema being validated against and the parsed document, so APM tools can associate the span with a concrete GraphQL operation. validate() is synchronous so traceSync is appropriate; errors thrown by assertValidSchema or the visitor rethrow propagate through the channel's error sub-channel. Also extracts the in-memory TracingChannel fake used by parser-diagnostics into src/__testUtils__/fakeDiagnosticsChannel.ts so subsequent emission sites (validate, execute, subscribe, resolve) can reuse it without duplicating the lifecycle simulation. --- integrationTests/diagnostics/test.js | 41 +++- src/__testUtils__/fakeDiagnosticsChannel.ts | 186 ++++++++++++++++++ .../__tests__/parser-diagnostics-test.ts | 181 +---------------- .../__tests__/validate-diagnostics-test.ts | 78 ++++++++ src/validation/validate.ts | 82 ++++---- 5 files changed, 359 insertions(+), 209 deletions(-) create mode 100644 src/__testUtils__/fakeDiagnosticsChannel.ts create mode 100644 src/validation/__tests__/validate-diagnostics-test.ts diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js index 646ceb7046..86661209ef 100644 --- a/integrationTests/diagnostics/test.js +++ b/integrationTests/diagnostics/test.js @@ -5,7 +5,12 @@ import assert from 'node:assert/strict'; import dc from 'node:diagnostics_channel'; -import { enableDiagnosticsChannel, parse } from 'graphql'; +import { + buildSchema, + enableDiagnosticsChannel, + parse, + validate, +} from 'graphql'; enableDiagnosticsChannel(dc); @@ -64,6 +69,40 @@ enableDiagnosticsChannel(dc); } } +// graphql:validate - synchronous, with schema/document context +{ + const schema = buildSchema(`type Query { field: String }`); + const doc = parse('{ field }'); + + const events = []; + const handler = { + start: (msg) => + events.push({ + kind: 'start', + schema: msg.schema, + document: msg.document, + }), + end: () => events.push({ kind: 'end' }), + error: (msg) => events.push({ kind: 'error', error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:validate'); + channel.subscribe(handler); + + try { + const errors = validate(schema, doc); + assert.deepEqual(errors, []); + assert.deepEqual( + events.map((e) => e.kind), + ['start', 'end'], + ); + assert.equal(events[0].schema, schema); + assert.equal(events[0].document, doc); + } finally { + channel.unsubscribe(handler); + } +} + // No-op when nothing is subscribed - parse still succeeds. { const doc = parse('{ field }'); diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts new file mode 100644 index 0000000000..47949f5c17 --- /dev/null +++ b/src/__testUtils__/fakeDiagnosticsChannel.ts @@ -0,0 +1,186 @@ +import type { + MinimalChannel, + MinimalDiagnosticsChannel, + MinimalTracingChannel, +} from '../diagnostics.js'; + +export type Listener = (message: unknown) => void; + +/** + * In-memory `MinimalChannel` implementation used by the unit tests. Tracks + * subscribers and replays Node's `runStores` semantics by simply invoking + * `fn`. + */ +export class FakeChannel implements MinimalChannel { + listeners: Array = []; + + get [Symbol.toStringTag]() { + return 'FakeChannel'; + } + + get hasSubscribers(): boolean { + return this.listeners.length > 0; + } + + publish(message: unknown): void { + for (const l of this.listeners) { + l(message); + } + } + + runStores( + _ctx: ContextType, + fn: (this: ContextType, ...args: Array) => T, + thisArg?: unknown, + ...args: Array + ): T { + return fn.apply(thisArg as ContextType, args); + } + + subscribe(listener: Listener): void { + this.listeners.push(listener); + } + + unsubscribe(listener: Listener): void { + const idx = this.listeners.indexOf(listener); + if (idx >= 0) { + this.listeners.splice(idx, 1); + } + } +} + +/** + * Structurally-faithful `MinimalTracingChannel` implementation mirroring + * Node's `TracingChannel.traceSync` / `tracePromise` lifecycle (start, + * runStores, error, asyncStart, asyncEnd, end). + */ +export class FakeTracingChannel implements MinimalTracingChannel { + start: FakeChannel = new FakeChannel(); + end: FakeChannel = new FakeChannel(); + asyncStart: FakeChannel = new FakeChannel(); + asyncEnd: FakeChannel = new FakeChannel(); + error: FakeChannel = new FakeChannel(); + + get [Symbol.toStringTag]() { + return 'FakeTracingChannel'; + } + + get hasSubscribers(): boolean { + return ( + this.start.hasSubscribers || + this.end.hasSubscribers || + this.asyncStart.hasSubscribers || + this.asyncEnd.hasSubscribers || + this.error.hasSubscribers + ); + } + + traceSync( + fn: (...args: Array) => T, + ctx: object, + thisArg?: unknown, + ...args: Array + ): T { + this.start.publish(ctx); + try { + return this.end.runStores(ctx, fn, thisArg, ...args); + } catch (err) { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + throw err; + } finally { + this.end.publish(ctx); + } + } + + tracePromise( + fn: (...args: Array) => Promise, + ctx: object, + thisArg?: unknown, + ...args: Array + ): Promise { + this.start.publish(ctx); + let promise: Promise; + try { + promise = this.end.runStores(ctx, fn, thisArg, ...args); + } catch (err) { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + this.end.publish(ctx); + throw err; + } + this.end.publish(ctx); + return promise.then( + (result) => { + (ctx as { result: unknown }).result = result; + this.asyncStart.publish(ctx); + this.asyncEnd.publish(ctx); + return result; + }, + (err: unknown) => { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + this.asyncStart.publish(ctx); + this.asyncEnd.publish(ctx); + throw err; + }, + ); + } +} + +export class FakeDc implements MinimalDiagnosticsChannel { + private cache = new Map(); + + get [Symbol.toStringTag]() { + return 'FakeDc'; + } + + tracingChannel(name: string): FakeTracingChannel { + let existing = this.cache.get(name); + if (existing === undefined) { + existing = new FakeTracingChannel(); + this.cache.set(name, existing); + } + return existing; + } +} + +export interface CollectedEvent { + kind: 'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error'; + ctx: { [key: string]: unknown }; +} + +/** + * Attach listeners to every sub-channel on a FakeTracingChannel and return + * the captured event buffer plus an unsubscribe hook. + */ +export function collectEvents(channel: FakeTracingChannel): { + events: Array; + unsubscribe: () => void; +} { + const events: Array = []; + const make = + (kind: CollectedEvent['kind']): Listener => + (m) => + events.push({ kind, ctx: m as { [key: string]: unknown } }); + const startL = make('start'); + const endL = make('end'); + const asyncStartL = make('asyncStart'); + const asyncEndL = make('asyncEnd'); + const errorL = make('error'); + channel.start.subscribe(startL); + channel.end.subscribe(endL); + channel.asyncStart.subscribe(asyncStartL); + channel.asyncEnd.subscribe(asyncEndL); + channel.error.subscribe(errorL); + return { + events, + unsubscribe() { + channel.start.unsubscribe(startL); + channel.end.unsubscribe(endL); + channel.asyncStart.unsubscribe(asyncStartL); + channel.asyncEnd.unsubscribe(asyncEndL); + channel.error.unsubscribe(errorL); + }, + }; +} diff --git a/src/language/__tests__/parser-diagnostics-test.ts b/src/language/__tests__/parser-diagnostics-test.ts index ac16f46bce..a970a73508 100644 --- a/src/language/__tests__/parser-diagnostics-test.ts +++ b/src/language/__tests__/parser-diagnostics-test.ts @@ -1,179 +1,18 @@ import { expect } from 'chai'; import { afterEach, beforeEach, describe, it } from 'mocha'; -import type { - MinimalChannel, - MinimalDiagnosticsChannel, - MinimalTracingChannel, -} from '../../diagnostics.js'; +import { + collectEvents, + FakeDc, +} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + import { enableDiagnosticsChannel } from '../../diagnostics.js'; import { parse } from '../parser.js'; -type Listener = (message: unknown) => void; - -class FakeChannel implements MinimalChannel { - listeners: Array = []; - get hasSubscribers(): boolean { - return this.listeners.length > 0; - } - - publish(message: unknown): void { - for (const l of this.listeners) { - l(message); - } - } - - runStores( - _ctx: ContextType, - fn: (this: ContextType, ...args: Array) => T, - thisArg?: unknown, - ...args: Array - ): T { - return fn.apply(thisArg as ContextType, args); - } - - subscribe(listener: Listener): void { - this.listeners.push(listener); - } - - unsubscribe(listener: Listener): void { - const idx = this.listeners.indexOf(listener); - if (idx >= 0) { - this.listeners.splice(idx, 1); - } - } -} - -class FakeTracingChannel implements MinimalTracingChannel { - start = new FakeChannel(); - end = new FakeChannel(); - asyncStart = new FakeChannel(); - asyncEnd = new FakeChannel(); - error = new FakeChannel(); - - get hasSubscribers(): boolean { - return ( - this.start.hasSubscribers || - this.end.hasSubscribers || - this.asyncStart.hasSubscribers || - this.asyncEnd.hasSubscribers || - this.error.hasSubscribers - ); - } - - traceSync( - fn: (...args: Array) => T, - ctx: object, - thisArg?: unknown, - ...args: Array - ): T { - this.start.publish(ctx); - try { - return this.end.runStores(ctx, fn, thisArg, ...args); - } catch (err) { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - throw err; - } finally { - this.end.publish(ctx); - } - } - - tracePromise( - fn: (...args: Array) => Promise, - ctx: object, - thisArg?: unknown, - ...args: Array - ): Promise { - this.start.publish(ctx); - let promise: Promise; - try { - promise = this.end.runStores(ctx, fn, thisArg, ...args); - } catch (err) { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - this.end.publish(ctx); - throw err; - } - this.end.publish(ctx); - return promise.then( - (result) => { - (ctx as { result: unknown }).result = result; - this.asyncStart.publish(ctx); - this.asyncEnd.publish(ctx); - return result; - }, - (err: unknown) => { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - this.asyncStart.publish(ctx); - this.asyncEnd.publish(ctx); - throw err; - }, - ); - } -} - -class FakeDc implements MinimalDiagnosticsChannel { - private cache = new Map(); - - tracingChannel(name: string): FakeTracingChannel { - let existing = this.cache.get(name); - if (existing === undefined) { - existing = new FakeTracingChannel(); - this.cache.set(name, existing); - } - return existing; - } -} - const fakeDc = new FakeDc(); const parseChannel = fakeDc.tracingChannel('graphql:parse'); -interface Event { - kind: 'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error'; - source: unknown; - error?: unknown; -} - -function collectEvents(): { events: Array; unsubscribe: () => void } { - const events: Array = []; - const startL: Listener = (m) => - events.push({ kind: 'start', source: (m as { source: unknown }).source }); - const endL: Listener = (m) => - events.push({ kind: 'end', source: (m as { source: unknown }).source }); - const asyncStartL: Listener = (m) => - events.push({ - kind: 'asyncStart', - source: (m as { source: unknown }).source, - }); - const asyncEndL: Listener = (m) => - events.push({ - kind: 'asyncEnd', - source: (m as { source: unknown }).source, - }); - const errorL: Listener = (m) => { - const msg = m as { source: unknown; error: unknown }; - events.push({ kind: 'error', source: msg.source, error: msg.error }); - }; - parseChannel.start.subscribe(startL); - parseChannel.end.subscribe(endL); - parseChannel.asyncStart.subscribe(asyncStartL); - parseChannel.asyncEnd.subscribe(asyncEndL); - parseChannel.error.subscribe(errorL); - return { - events, - unsubscribe() { - parseChannel.start.unsubscribe(startL); - parseChannel.end.unsubscribe(endL); - parseChannel.asyncStart.unsubscribe(asyncStartL); - parseChannel.asyncEnd.unsubscribe(asyncEndL); - parseChannel.error.unsubscribe(errorL); - }, - }; -} - describe('parse diagnostics channel', () => { let active: ReturnType | undefined; @@ -187,24 +26,24 @@ describe('parse diagnostics channel', () => { }); it('emits start and end around a successful parse', () => { - active = collectEvents(); + active = collectEvents(parseChannel); const doc = parse('{ field }'); expect(doc.kind).to.equal('Document'); expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); - expect(active.events[0].source).to.equal('{ field }'); - expect(active.events[1].source).to.equal('{ field }'); + expect(active.events[0].ctx.source).to.equal('{ field }'); + expect(active.events[1].ctx.source).to.equal('{ field }'); }); it('emits start, error, and end when the parser throws', () => { - active = collectEvents(); + active = collectEvents(parseChannel); expect(() => parse('{ ')).to.throw(); const kinds = active.events.map((e) => e.kind); expect(kinds).to.deep.equal(['start', 'error', 'end']); - expect(active.events[1].error).to.be.instanceOf(Error); + expect(active.events[1].ctx.error).to.be.instanceOf(Error); }); it('does nothing when no subscribers are attached', () => { diff --git a/src/validation/__tests__/validate-diagnostics-test.ts b/src/validation/__tests__/validate-diagnostics-test.ts new file mode 100644 index 0000000000..28d1c40c3b --- /dev/null +++ b/src/validation/__tests__/validate-diagnostics-test.ts @@ -0,0 +1,78 @@ +import { expect } from 'chai'; +import { afterEach, beforeEach, describe, it } from 'mocha'; + +import { + collectEvents, + FakeDc, +} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + +import { parse } from '../../language/parser.js'; + +import { buildSchema } from '../../utilities/buildASTSchema.js'; + +import { enableDiagnosticsChannel } from '../../diagnostics.js'; + +import { validate } from '../validate.js'; + +const schema = buildSchema(` + type Query { + field: String + } +`); + +const fakeDc = new FakeDc(); +const validateChannel = fakeDc.tracingChannel('graphql:validate'); + +describe('validate diagnostics channel', () => { + let active: ReturnType | undefined; + + beforeEach(() => { + enableDiagnosticsChannel(fakeDc); + }); + + afterEach(() => { + active?.unsubscribe(); + active = undefined; + }); + + it('emits start and end around a successful validate', () => { + active = collectEvents(validateChannel); + + const doc = parse('{ field }'); + const errors = validate(schema, doc); + + expect(errors).to.deep.equal([]); + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + expect(active.events[0].ctx.schema).to.equal(schema); + expect(active.events[0].ctx.document).to.equal(doc); + }); + + it('emits start and end for a document with validation errors', () => { + active = collectEvents(validateChannel); + + const doc = parse('{ missingField }'); + const errors = validate(schema, doc); + + expect(errors).to.have.length.greaterThan(0); + // Validation errors are collected, not thrown, so we still see start/end. + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + }); + + it('emits start, error, and end when validate throws on an invalid schema', () => { + active = collectEvents(validateChannel); + + expect(() => validate({} as typeof schema, parse('{ field }'))).to.throw(); + + expect(active.events.map((e) => e.kind)).to.deep.equal([ + 'start', + 'error', + 'end', + ]); + expect(active.events[1].ctx.error).to.be.instanceOf(Error); + }); + + it('does nothing when no subscribers are attached', () => { + const errors = validate(schema, parse('{ field }')); + expect(errors).to.deep.equal([]); + }); +}); diff --git a/src/validation/validate.ts b/src/validation/validate.ts index 544f8bc3aa..0c6a7499e6 100644 --- a/src/validation/validate.ts +++ b/src/validation/validate.ts @@ -12,6 +12,8 @@ import { assertValidSchema } from '../type/validate.js'; import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js'; +import { maybeTraceSync } from '../diagnostics.js'; + import { specifiedRules, specifiedSDLRules } from './specifiedRules.js'; import type { SDLValidationRule, ValidationRule } from './ValidationContext.js'; import { @@ -61,46 +63,52 @@ export function validate( rules: ReadonlyArray = specifiedRules, options?: ValidationOptions, ): ReadonlyArray { - const maxErrors = options?.maxErrors ?? 100; - const hideSuggestions = options?.hideSuggestions ?? false; - - // If the schema used for validation is invalid, throw an error. - assertValidSchema(schema); - - const errors: Array = []; - const typeInfo = new TypeInfo(schema); - const context = new ValidationContext( - schema, - documentAST, - typeInfo, - (error) => { - if (errors.length >= maxErrors) { - throw tooManyValidationErrorsError; + return maybeTraceSync( + 'validate', + () => ({ schema, document: documentAST }), + () => { + const maxErrors = options?.maxErrors ?? 100; + const hideSuggestions = options?.hideSuggestions ?? false; + + // If the schema used for validation is invalid, throw an error. + assertValidSchema(schema); + + const errors: Array = []; + const typeInfo = new TypeInfo(schema); + const context = new ValidationContext( + schema, + documentAST, + typeInfo, + (error) => { + if (errors.length >= maxErrors) { + throw tooManyValidationErrorsError; + } + errors.push(error); + }, + hideSuggestions, + ); + + // This uses a specialized visitor which runs multiple visitors in + // parallel, while maintaining the visitor skip and break API. + const visitor = visitInParallel(rules.map((rule) => rule(context))); + + // Visit the whole document with each instance of all provided rules. + try { + visit( + documentAST, + visitWithTypeInfo(typeInfo, visitor), + QueryDocumentKeysToValidate, + ); + } catch (e: unknown) { + if (e === tooManyValidationErrorsError) { + errors.push(tooManyValidationErrorsError); + } else { + throw e; + } } - errors.push(error); + return errors; }, - hideSuggestions, ); - - // This uses a specialized visitor which runs multiple visitors in parallel, - // while maintaining the visitor skip and break API. - const visitor = visitInParallel(rules.map((rule) => rule(context))); - - // Visit the whole document with each instance of all provided rules. - try { - visit( - documentAST, - visitWithTypeInfo(typeInfo, visitor), - QueryDocumentKeysToValidate, - ); - } catch (e: unknown) { - if (e === tooManyValidationErrorsError) { - errors.push(tooManyValidationErrorsError); - } else { - throw e; - } - } - return errors; } /** From abc84dbc8a5bf3dedc0a580333004bb7cc669758 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 15:39:18 -0400 Subject: [PATCH 05/29] feat(execution): publish on graphql:execute tracing channel Wraps the public execute / experimentalExecuteIncrementally / executeIgnoringIncremental / executeSubscriptionEvent entry points with maybeTraceMixed so subscribers see a single graphql:execute span per top-level operation invocation, including every per-event execution of a subscription stream. The context exposes the document, schema, variableValues, operationName, and operationType. operationName and operationType are lazy getters that only resolve the operation AST (getOperationAST) if a subscriber reads them, keeping the gate cheap for APMs that do not need them. The ValidatedExecutionArgs variant used by executeSubscriptionEvent uses the already-resolved operation from validation and therefore needs no lazy lookup; it carries the resolved operation in place of the (unavailable) document. Adds a shared maybeTraceMixed helper to src/diagnostics.ts for PromiseOrValue-returning functions. It delegates start / end / error to Node's traceSync (which also runs fn inside end.runStores for AsyncLocalStorage propagation) and adds the asyncStart / asyncEnd / error-on-rejection emission when the return value is a promise. --- integrationTests/diagnostics/test.js | 46 ++++++ src/diagnostics.ts | 51 +++++++ .../__tests__/execute-diagnostics-test.ts | 139 ++++++++++++++++++ src/execution/execute.ts | 128 ++++++++++++---- 4 files changed, 332 insertions(+), 32 deletions(-) create mode 100644 src/execution/__tests__/execute-diagnostics-test.ts diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js index 86661209ef..7013070afc 100644 --- a/integrationTests/diagnostics/test.js +++ b/integrationTests/diagnostics/test.js @@ -8,6 +8,7 @@ import dc from 'node:diagnostics_channel'; import { buildSchema, enableDiagnosticsChannel, + execute, parse, validate, } from 'graphql'; @@ -103,6 +104,51 @@ enableDiagnosticsChannel(dc); } } +// graphql:execute - sync path, ctx carries operationType, operationName, +// document, schema. +{ + const schema = buildSchema(`type Query { hello: String }`); + const document = parse('query Greeting { hello }'); + + const events = []; + const handler = { + start: (msg) => + events.push({ + kind: 'start', + operationType: msg.operationType, + operationName: msg.operationName, + document: msg.document, + schema: msg.schema, + }), + end: () => events.push({ kind: 'end' }), + asyncStart: () => events.push({ kind: 'asyncStart' }), + asyncEnd: () => events.push({ kind: 'asyncEnd' }), + error: (msg) => events.push({ kind: 'error', error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:execute'); + channel.subscribe(handler); + + try { + const result = execute({ + schema, + document, + rootValue: { hello: 'world' }, + }); + assert.equal(result.data.hello, 'world'); + assert.deepEqual( + events.map((e) => e.kind), + ['start', 'end'], + ); + assert.equal(events[0].operationType, 'query'); + assert.equal(events[0].operationName, 'Greeting'); + assert.equal(events[0].document, document); + assert.equal(events[0].schema, schema); + } finally { + channel.unsubscribe(handler); + } +} + // No-op when nothing is subscribed - parse still succeeds. { const doc = parse('{ field }'); diff --git a/src/diagnostics.ts b/src/diagnostics.ts index b37c3f4a8e..d8551963d8 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -13,6 +13,8 @@ * same `TracingChannel` instances and all subscribers coexist. */ +import { isPromise } from './jsutils/isPromise.js'; + /** * Structural subset of `DiagnosticsChannel` sufficient for publishing and * subscriber gating. `node:diagnostics_channel`'s `Channel` satisfies this. @@ -181,3 +183,52 @@ export function maybeTracePromise( } return channel.tracePromise(fn, ctxFactory()); } + +/** + * Publish a mixed sync-or-promise operation through the named graphql tracing + * channel. Delegates the start/end/error lifecycle to Node's `traceSync` + * (which also runs `fn` inside `end.runStores` for AsyncLocalStorage context + * propagation) and, when `fn` returns a promise, appends `asyncStart` and + * `asyncEnd` on settlement plus `error` on rejection. + * + * Use this when the function may return either a value or a promise, which + * is common on graphql-js's execution path where async-ness is determined + * by resolvers only after the call begins. Short-circuits to `fn()` when + * the channel isn't registered or nothing is listening. + * + * @internal + */ +export function maybeTraceMixed( + name: keyof GraphQLChannels, + ctxFactory: () => object, + fn: () => T | Promise, +): T | Promise { + const channel = getChannels()?.[name]; + if (!shouldTrace(channel)) { + return fn(); + } + const ctx = ctxFactory() as { + error?: unknown; + result?: unknown; + }; + const result = channel.traceSync(fn, ctx); + if (!isPromise(result)) { + ctx.result = result; + return result; + } + return result.then( + (value) => { + ctx.result = value; + channel.asyncStart.publish(ctx); + channel.asyncEnd.publish(ctx); + return value; + }, + (err: unknown) => { + ctx.error = err; + channel.error.publish(ctx); + channel.asyncStart.publish(ctx); + channel.asyncEnd.publish(ctx); + throw err; + }, + ); +} diff --git a/src/execution/__tests__/execute-diagnostics-test.ts b/src/execution/__tests__/execute-diagnostics-test.ts new file mode 100644 index 0000000000..45fcbdee3c --- /dev/null +++ b/src/execution/__tests__/execute-diagnostics-test.ts @@ -0,0 +1,139 @@ +import { expect } from 'chai'; +import { afterEach, beforeEach, describe, it } from 'mocha'; + +import { + collectEvents, + FakeDc, +} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + +import { parse } from '../../language/parser.js'; + +import { buildSchema } from '../../utilities/buildASTSchema.js'; + +import { enableDiagnosticsChannel } from '../../diagnostics.js'; + +import type { ExecutionArgs } from '../execute.js'; +import { + execute, + executeSubscriptionEvent, + executeSync, + validateExecutionArgs, +} from '../execute.js'; + +const schema = buildSchema(` + type Query { + sync: String + async: String + } +`); + +const rootValue = { + sync: () => 'hello', + async: () => Promise.resolve('hello-async'), +}; + +const fakeDc = new FakeDc(); +const executeChannel = fakeDc.tracingChannel('graphql:execute'); + +describe('execute diagnostics channel', () => { + let active: ReturnType | undefined; + + beforeEach(() => { + enableDiagnosticsChannel(fakeDc); + }); + + afterEach(() => { + active?.unsubscribe(); + active = undefined; + }); + + it('emits start and end around a synchronous execute', () => { + active = collectEvents(executeChannel); + + const document = parse('query Q { sync }'); + const result = execute({ schema, document, rootValue }); + + expect(result).to.deep.equal({ data: { sync: 'hello' } }); + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + expect(active.events[0].ctx.operationType).to.equal('query'); + expect(active.events[0].ctx.operationName).to.equal('Q'); + expect(active.events[0].ctx.document).to.equal(document); + expect(active.events[0].ctx.schema).to.equal(schema); + }); + + it('emits start, end, and async lifecycle when execute returns a promise', async () => { + active = collectEvents(executeChannel); + + const document = parse('query { async }'); + const result = await execute({ schema, document, rootValue }); + + expect(result).to.deep.equal({ data: { async: 'hello-async' } }); + expect(active.events.map((e) => e.kind)).to.deep.equal([ + 'start', + 'end', + 'asyncStart', + 'asyncEnd', + ]); + }); + + it('emits once for executeSync via experimentalExecuteIncrementally', () => { + active = collectEvents(executeChannel); + + const document = parse('{ sync }'); + executeSync({ schema, document, rootValue }); + + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + }); + + it('emits start, error, and end when execute throws synchronously', () => { + active = collectEvents(executeChannel); + + const schemaWithDefer = buildSchema(` + directive @defer on FIELD + type Query { sync: String } + `); + const document = parse('{ sync }'); + expect(() => + execute({ schema: schemaWithDefer, document, rootValue }), + ).to.throw(); + + expect(active.events.map((e) => e.kind)).to.deep.equal([ + 'start', + 'error', + 'end', + ]); + }); + + it('emits for each executeSubscriptionEvent call with resolved operation ctx', () => { + const args: ExecutionArgs = { + schema, + document: parse('query Q { sync }'), + rootValue, + }; + const validated = validateExecutionArgs(args); + if (!('schema' in validated)) { + throw new Error('unexpected validation failure'); + } + + active = collectEvents(executeChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + executeSubscriptionEvent(validated); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + executeSubscriptionEvent(validated); + + const starts = active.events.filter((e) => e.kind === 'start'); + expect(starts.length).to.equal(2); + for (const ev of starts) { + expect(ev.ctx.operationType).to.equal('query'); + expect(ev.ctx.operationName).to.equal('Q'); + expect(ev.ctx.schema).to.equal(schema); + } + }); + + it('does nothing when no subscribers are attached', () => { + const document = parse('{ sync }'); + const result = execute({ schema, document, rootValue }); + expect(result).to.deep.equal({ data: { sync: 'hello' } }); + }); +}); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 4d71b30e85..830595d137 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -32,6 +32,10 @@ import type { import { assertValidSchema } from '../type/index.js'; import type { GraphQLSchema } from '../type/schema.js'; +import { getOperationAST } from '../utilities/getOperationAST.js'; + +import { maybeTraceMixed } from '../diagnostics.js'; + import { cancellablePromise } from './cancellablePromise.js'; import type { FieldDetailsList, FragmentDetails } from './collectFields.js'; import { collectFields } from './collectFields.js'; @@ -49,6 +53,53 @@ import { getArgumentValues, getVariableValues } from './values.js'; const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = 'The provided schema unexpectedly contains experimental directives (@defer or @stream). These directives may only be utilized if experimental execution features are explicitly enabled.'; +/** + * Build a graphql:execute channel context from raw ExecutionArgs. Defers + * resolution of the operation AST to a lazy getter so the cost of walking + * the document is only paid if a subscriber reads it. + */ +function buildExecuteCtxFromArgs(args: ExecutionArgs): () => object { + return () => { + let operation: OperationDefinitionNode | null | undefined; + const resolveOperation = (): OperationDefinitionNode | null | undefined => { + if (operation === undefined) { + operation = getOperationAST(args.document, args.operationName); + } + return operation; + }; + return { + document: args.document, + schema: args.schema, + variableValues: args.variableValues, + get operationName() { + return args.operationName ?? resolveOperation()?.name?.value; + }, + get operationType() { + return resolveOperation()?.operation; + }, + }; + }; +} + +/** + * Build a graphql:execute channel context from ValidatedExecutionArgs. + * Used by executeSubscriptionEvent, where the operation has already been + * resolved during argument validation. The original document is not + * available at this point, only the resolved operation; subscribers that + * need the document should read it from the graphql:subscribe context. + */ +function buildExecuteCtxFromValidatedArgs( + args: ValidatedExecutionArgs, +): () => object { + return () => ({ + operation: args.operation, + schema: args.schema, + variableValues: args.variableValues, + operationName: args.operation.name?.value, + operationType: args.operation.operation, + }); +} + /** * Implements the "Executing requests" section of the GraphQL specification. * @@ -66,18 +117,23 @@ const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = * delivery. */ export function execute(args: ExecutionArgs): PromiseOrValue { - if (args.schema.getDirective('defer') || args.schema.getDirective('stream')) { - throw new Error(UNEXPECTED_EXPERIMENTAL_DIRECTIVES); - } + return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => { + if ( + args.schema.getDirective('defer') || + args.schema.getDirective('stream') + ) { + throw new Error(UNEXPECTED_EXPERIMENTAL_DIRECTIVES); + } - const validatedExecutionArgs = validateExecutionArgs(args); + const validatedExecutionArgs = validateExecutionArgs(args); - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } - return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); + return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); + }); } /** @@ -95,35 +151,39 @@ export function execute(args: ExecutionArgs): PromiseOrValue { export function experimentalExecuteIncrementally( args: ExecutionArgs, ): PromiseOrValue { - // If a valid execution context cannot be created due to incorrect arguments, - // a "Response" with only errors is returned. - const validatedExecutionArgs = validateExecutionArgs(args); - - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => { + // If a valid execution context cannot be created due to incorrect + // arguments, a "Response" with only errors is returned. + const validatedExecutionArgs = validateExecutionArgs(args); + + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } - return experimentalExecuteQueryOrMutationOrSubscriptionEvent( - validatedExecutionArgs, - ); + return experimentalExecuteQueryOrMutationOrSubscriptionEvent( + validatedExecutionArgs, + ); + }); } export function executeIgnoringIncremental( args: ExecutionArgs, ): PromiseOrValue { - // If a valid execution context cannot be created due to incorrect arguments, - // a "Response" with only errors is returned. - const validatedExecutionArgs = validateExecutionArgs(args); - - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => { + // If a valid execution context cannot be created due to incorrect + // arguments, a "Response" with only errors is returned. + const validatedExecutionArgs = validateExecutionArgs(args); + + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } - return executeQueryOrMutationOrSubscriptionEventIgnoringIncremental( - validatedExecutionArgs, - ); + return executeQueryOrMutationOrSubscriptionEventIgnoringIncremental( + validatedExecutionArgs, + ); + }); } /** @@ -184,7 +244,11 @@ export function executeSync(args: ExecutionArgs): ExecutionResult { export function executeSubscriptionEvent( validatedExecutionArgs: ValidatedExecutionArgs, ): PromiseOrValue { - return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); + return maybeTraceMixed( + 'execute', + buildExecuteCtxFromValidatedArgs(validatedExecutionArgs), + () => executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs), + ); } /** From d3f2943702845704eac5ecd6159e0def6dd7f5a3 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 16:07:33 -0400 Subject: [PATCH 06/29] refactor(diagnostics): align async lifecycle with Node's tracePromise shape maybeTraceMixed now publishes asyncStart immediately once it knows the operation is in-flight asynchronously and asyncEnd in a .finally once the promise settles, matching the shape subscribers expect from a tracePromise-style channel (asyncStart brackets the async tail rather than being paired with asyncEnd in a single microtask). Also brings the in-memory FakeTracingChannel in line with Node's actual traceSync behavior: Node sets ctx.result before publishing end, letting subscribers check isPromise(ctx.result) inside their end handler to decide whether asyncEnd will follow or the span is complete. The fake now does the same so unit tests aren't looser than real-Node behavior. --- src/__testUtils__/fakeDiagnosticsChannel.ts | 42 ++++++++++-------- src/diagnostics.ts | 47 ++++++++++----------- 2 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts index 47949f5c17..4a0bcd7f84 100644 --- a/src/__testUtils__/fakeDiagnosticsChannel.ts +++ b/src/__testUtils__/fakeDiagnosticsChannel.ts @@ -82,15 +82,22 @@ export class FakeTracingChannel implements MinimalTracingChannel { ...args: Array ): T { this.start.publish(ctx); + let result: T; try { - return this.end.runStores(ctx, fn, thisArg, ...args); + result = this.end.runStores(ctx, fn, thisArg, ...args); } catch (err) { (ctx as { error: unknown }).error = err; this.error.publish(ctx); - throw err; - } finally { this.end.publish(ctx); + throw err; } + // Node's real traceSync sets `ctx.result` before publishing `end`, so + // subscribers can inspect `isPromise(ctx.result)` inside their `end` + // handler to decide whether the operation is complete or async events + // will follow. Match that semantic here. + (ctx as { result: unknown }).result = result; + this.end.publish(ctx); + return result; } tracePromise( @@ -110,21 +117,22 @@ export class FakeTracingChannel implements MinimalTracingChannel { throw err; } this.end.publish(ctx); - return promise.then( - (result) => { - (ctx as { result: unknown }).result = result; - this.asyncStart.publish(ctx); + this.asyncStart.publish(ctx); + return promise + .then( + (result) => { + (ctx as { result: unknown }).result = result; + return result; + }, + (err: unknown) => { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + throw err; + }, + ) + .finally(() => { this.asyncEnd.publish(ctx); - return result; - }, - (err: unknown) => { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - this.asyncStart.publish(ctx); - this.asyncEnd.publish(ctx); - throw err; - }, - ); + }); } } diff --git a/src/diagnostics.ts b/src/diagnostics.ts index d8551963d8..4971fc4fd2 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -185,16 +185,7 @@ export function maybeTracePromise( } /** - * Publish a mixed sync-or-promise operation through the named graphql tracing - * channel. Delegates the start/end/error lifecycle to Node's `traceSync` - * (which also runs `fn` inside `end.runStores` for AsyncLocalStorage context - * propagation) and, when `fn` returns a promise, appends `asyncStart` and - * `asyncEnd` on settlement plus `error` on rejection. - * - * Use this when the function may return either a value or a promise, which - * is common on graphql-js's execution path where async-ness is determined - * by resolvers only after the call begins. Short-circuits to `fn()` when - * the channel isn't registered or nothing is listening. + * Publish a mixed sync-or-promise operation through the named graphql tracing channel. * * @internal */ @@ -211,24 +202,30 @@ export function maybeTraceMixed( error?: unknown; result?: unknown; }; + + // traceSync fires start/end (and error, if fn throws synchronously) const result = channel.traceSync(fn, ctx); if (!isPromise(result)) { - ctx.result = result; return result; } - return result.then( - (value) => { - ctx.result = value; - channel.asyncStart.publish(ctx); - channel.asyncEnd.publish(ctx); - return value; - }, - (err: unknown) => { - ctx.error = err; - channel.error.publish(ctx); - channel.asyncStart.publish(ctx); + + // Fires off `asyncStart` and `asyncEnd` lifecycle events. + channel.asyncStart.publish(ctx); + return result + .then( + (value) => { + ctx.result = value; + + return value; + }, + (err: unknown) => { + ctx.error = err; + channel.error.publish(ctx); + + throw err; + }, + ) + .finally(() => { channel.asyncEnd.publish(ctx); - throw err; - }, - ); + }); } From 2e9ebd5ca93389c49252b0e86a5deeaa55a5c079 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 16:17:53 -0400 Subject: [PATCH 07/29] feat(execution): publish on graphql:subscribe tracing channel Wraps the public subscribe() entry point with maybeTraceMixed using the same ExecutionArgs-based context factory as graphql:execute. The subscription setup path may return sync (when the subscribe resolver is synchronous) or async (when the resolver returns a promise resolving to an AsyncIterable), so maybeTraceMixed's sync / promise branching fits exactly. Per-subscription-event executions continue to publish on graphql:execute via executeSubscriptionEvent, which was wired in the previous commit. The graphql:subscribe context therefore owns the document reference and covers the setup span; subscribers that need to correlate per-event execute spans to their parent subscription use AsyncLocalStorage context propagation provided by Channel.runStores. --- integrationTests/diagnostics/test.js | 173 ++++++++++++------ .../__tests__/subscribe-diagnostics-test.ts | 130 +++++++++++++ src/execution/execute.ts | 30 +-- 3 files changed, 261 insertions(+), 72 deletions(-) create mode 100644 src/execution/__tests__/subscribe-diagnostics-test.ts diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js index 7013070afc..ae25b6ef64 100644 --- a/integrationTests/diagnostics/test.js +++ b/integrationTests/diagnostics/test.js @@ -10,68 +10,70 @@ import { enableDiagnosticsChannel, execute, parse, + subscribe, validate, } from 'graphql'; enableDiagnosticsChannel(dc); -// graphql:parse - synchronous -{ - const events = []; - const handler = { - start: (msg) => events.push({ kind: 'start', source: msg.source }), - end: (msg) => events.push({ kind: 'end', source: msg.source }), - asyncStart: (msg) => - events.push({ kind: 'asyncStart', source: msg.source }), - asyncEnd: (msg) => events.push({ kind: 'asyncEnd', source: msg.source }), - error: (msg) => - events.push({ kind: 'error', source: msg.source, error: msg.error }), - }; - - const channel = dc.tracingChannel('graphql:parse'); - channel.subscribe(handler); - - try { - const doc = parse('{ field }'); - assert.equal(doc.kind, 'Document'); - assert.deepEqual( - events.map((e) => e.kind), - ['start', 'end'], - ); - assert.equal(events[0].source, '{ field }'); - assert.equal(events[1].source, '{ field }'); - } finally { - channel.unsubscribe(handler); +function runParseCases() { + // graphql:parse - synchronous. + { + const events = []; + const handler = { + start: (msg) => events.push({ kind: 'start', source: msg.source }), + end: (msg) => events.push({ kind: 'end', source: msg.source }), + asyncStart: (msg) => + events.push({ kind: 'asyncStart', source: msg.source }), + asyncEnd: (msg) => events.push({ kind: 'asyncEnd', source: msg.source }), + error: (msg) => + events.push({ kind: 'error', source: msg.source, error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:parse'); + channel.subscribe(handler); + + try { + const doc = parse('{ field }'); + assert.equal(doc.kind, 'Document'); + assert.deepEqual( + events.map((e) => e.kind), + ['start', 'end'], + ); + assert.equal(events[0].source, '{ field }'); + assert.equal(events[1].source, '{ field }'); + } finally { + channel.unsubscribe(handler); + } } -} -// graphql:parse - error path fires start, error, end (traceSync finally-emits end) -{ - const events = []; - const handler = { - start: (msg) => events.push({ kind: 'start', source: msg.source }), - end: (msg) => events.push({ kind: 'end', source: msg.source }), - error: (msg) => - events.push({ kind: 'error', source: msg.source, error: msg.error }), - }; - - const channel = dc.tracingChannel('graphql:parse'); - channel.subscribe(handler); - - try { - assert.throws(() => parse('{ ')); - assert.deepEqual( - events.map((e) => e.kind), - ['start', 'error', 'end'], - ); - assert.ok(events[1].error instanceof Error); - } finally { - channel.unsubscribe(handler); + // graphql:parse - error path fires start, error, end. + { + const events = []; + const handler = { + start: (msg) => events.push({ kind: 'start', source: msg.source }), + end: (msg) => events.push({ kind: 'end', source: msg.source }), + error: (msg) => + events.push({ kind: 'error', source: msg.source, error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:parse'); + channel.subscribe(handler); + + try { + assert.throws(() => parse('{ ')); + assert.deepEqual( + events.map((e) => e.kind), + ['start', 'error', 'end'], + ); + assert.ok(events[1].error instanceof Error); + } finally { + channel.unsubscribe(handler); + } } } -// graphql:validate - synchronous, with schema/document context -{ +function runValidateCase() { const schema = buildSchema(`type Query { field: String }`); const doc = parse('{ field }'); @@ -104,9 +106,7 @@ enableDiagnosticsChannel(dc); } } -// graphql:execute - sync path, ctx carries operationType, operationName, -// document, schema. -{ +function runExecuteCase() { const schema = buildSchema(`type Query { hello: String }`); const document = parse('query Greeting { hello }'); @@ -149,10 +149,67 @@ enableDiagnosticsChannel(dc); } } -// No-op when nothing is subscribed - parse still succeeds. -{ +async function runSubscribeCase() { + async function* ticks() { + yield { tick: 'one' }; + } + + const schema = buildSchema(` + type Query { dummy: String } + type Subscription { tick: String } + `); + // buildSchema doesn't attach a subscribe resolver to fields; inject one. + schema.getSubscriptionType().getFields().tick.subscribe = () => ticks(); + + const document = parse('subscription Tick { tick }'); + + const events = []; + const handler = { + start: (msg) => + events.push({ + kind: 'start', + operationType: msg.operationType, + operationName: msg.operationName, + }), + end: () => events.push({ kind: 'end' }), + asyncStart: () => events.push({ kind: 'asyncStart' }), + asyncEnd: () => events.push({ kind: 'asyncEnd' }), + error: (msg) => events.push({ kind: 'error', error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:subscribe'); + channel.subscribe(handler); + + try { + const result = subscribe({ schema, document }); + const stream = typeof result.then === 'function' ? await result : result; + if (stream[Symbol.asyncIterator]) { + await stream.return?.(); + } + // Subscription setup is synchronous here; start/end fire, no async tail. + assert.deepEqual( + events.map((e) => e.kind), + ['start', 'end'], + ); + assert.equal(events[0].operationType, 'subscription'); + assert.equal(events[0].operationName, 'Tick'); + } finally { + channel.unsubscribe(handler); + } +} + +function runNoSubscriberCase() { const doc = parse('{ field }'); assert.equal(doc.kind, 'Document'); } -console.log('diagnostics integration test passed'); +async function main() { + runParseCases(); + runValidateCase(); + runExecuteCase(); + await runSubscribeCase(); + runNoSubscriberCase(); + console.log('diagnostics integration test passed'); +} + +main(); diff --git a/src/execution/__tests__/subscribe-diagnostics-test.ts b/src/execution/__tests__/subscribe-diagnostics-test.ts new file mode 100644 index 0000000000..a1b1987ecb --- /dev/null +++ b/src/execution/__tests__/subscribe-diagnostics-test.ts @@ -0,0 +1,130 @@ +import { expect } from 'chai'; +import { afterEach, beforeEach, describe, it } from 'mocha'; + +import { + collectEvents, + FakeDc, +} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + +import { isPromise } from '../../jsutils/isPromise.js'; + +import { parse } from '../../language/parser.js'; + +import { GraphQLObjectType } from '../../type/definition.js'; +import { GraphQLString } from '../../type/scalars.js'; +import { GraphQLSchema } from '../../type/schema.js'; + +import { enableDiagnosticsChannel } from '../../diagnostics.js'; + +import { subscribe } from '../execute.js'; + +function buildSubscriptionSchema( + subscribeFn: () => AsyncIterable<{ tick: string }>, +): GraphQLSchema { + return new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { dummy: { type: GraphQLString } }, + }), + subscription: new GraphQLObjectType({ + name: 'Subscription', + fields: { + tick: { + type: GraphQLString, + subscribe: subscribeFn, + }, + }, + }), + }); +} + +async function* twoTicks(): AsyncIterable<{ tick: string }> { + await Promise.resolve(); + yield { tick: 'one' }; + yield { tick: 'two' }; +} + +const fakeDc = new FakeDc(); +const subscribeChannel = fakeDc.tracingChannel('graphql:subscribe'); + +describe('subscribe diagnostics channel', () => { + let active: ReturnType | undefined; + + beforeEach(() => { + enableDiagnosticsChannel(fakeDc); + }); + + afterEach(() => { + active?.unsubscribe(); + active = undefined; + }); + + it('emits start and end for a synchronous subscription setup', async () => { + active = collectEvents(subscribeChannel); + + const schema = buildSubscriptionSchema(twoTicks); + const document = parse('subscription S { tick }'); + + const result = subscribe({ schema, document }); + const resolved = isPromise(result) ? await result : result; + if (!(Symbol.asyncIterator in resolved)) { + throw new Error('Expected an async iterator'); + } + await resolved.return?.(); + + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + expect(active.events[0].ctx.operationType).to.equal('subscription'); + expect(active.events[0].ctx.operationName).to.equal('S'); + expect(active.events[0].ctx.document).to.equal(document); + expect(active.events[0].ctx.schema).to.equal(schema); + }); + + it('emits the full async lifecycle when subscribe resolver returns a promise', async () => { + active = collectEvents(subscribeChannel); + + const asyncResolver = (): Promise> => + Promise.resolve(twoTicks()); + const schema = buildSubscriptionSchema( + asyncResolver as unknown as () => AsyncIterable<{ tick: string }>, + ); + const document = parse('subscription { tick }'); + + const result = subscribe({ schema, document }); + const resolved = isPromise(result) ? await result : result; + if (!(Symbol.asyncIterator in resolved)) { + throw new Error('Expected an async iterator'); + } + await resolved.return?.(); + + expect(active.events.map((e) => e.kind)).to.deep.equal([ + 'start', + 'end', + 'asyncStart', + 'asyncEnd', + ]); + }); + + it('emits only start and end for a synchronous validation failure', () => { + active = collectEvents(subscribeChannel); + + const schema = buildSubscriptionSchema(twoTicks); + // Invalid: no operation. + const document = parse('fragment F on Subscription { tick }'); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + subscribe({ schema, document }); + + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + }); + + it('does nothing when no subscribers are attached', async () => { + const schema = buildSubscriptionSchema(twoTicks); + const document = parse('subscription { tick }'); + + const result = subscribe({ schema, document }); + const resolved = isPromise(result) ? await result : result; + if (Symbol.asyncIterator in resolved) { + await resolved.return?.(); + } + }); +}); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 830595d137..c818401850 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -282,24 +282,26 @@ export function subscribe( ): PromiseOrValue< AsyncGenerator | ExecutionResult > { - // If a valid execution context cannot be created due to incorrect arguments, - // a "Response" with only errors is returned. - const validatedExecutionArgs = validateExecutionArgs(args); + return maybeTraceMixed('subscribe', buildExecuteCtxFromArgs(args), () => { + // If a valid execution context cannot be created due to incorrect + // arguments, a "Response" with only errors is returned. + const validatedExecutionArgs = validateExecutionArgs(args); - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } - const resultOrStream = createSourceEventStreamImpl(validatedExecutionArgs); + const resultOrStream = createSourceEventStreamImpl(validatedExecutionArgs); - if (isPromise(resultOrStream)) { - return resultOrStream.then((resolvedResultOrStream) => - mapSourceToResponse(validatedExecutionArgs, resolvedResultOrStream), - ); - } + if (isPromise(resultOrStream)) { + return resultOrStream.then((resolvedResultOrStream) => + mapSourceToResponse(validatedExecutionArgs, resolvedResultOrStream), + ); + } - return mapSourceToResponse(validatedExecutionArgs, resultOrStream); + return mapSourceToResponse(validatedExecutionArgs, resultOrStream); + }); } /** From ec51b09bdf7b9f1a7dda419997705a954ea4cb85 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 16:27:00 -0400 Subject: [PATCH 08/29] feat(execution): publish on graphql:resolve tracing channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps the single resolver invocation site in Executor.executeField with maybeTraceMixed so each field resolver call emits start/end (and the async tail when the resolver returns a promise, or error when it throws). The emission is inside the engine — no field.resolve mutation, no schema walking, nothing stacks across multiple APM subscribers. The published context captures the field-level metadata APM tools use to name and attribute per-field spans: - fieldName from info.fieldName - parentType from info.parentType.name - fieldType stringified info.returnType - args resolver arguments, by reference - isTrivialResolver (field.resolve === undefined), a hint for subscribers that want to skip default property-access resolvers - fieldPath lazy getter, serializes info.path only on first read since it is O(depth) work and many subscribers depth-filter without reading it graphql:resolve is the noisiest channel (one event per field per operation); all emission sites are gated on hasSubscribers before any context is constructed, so it remains zero-cost when no APM is loaded. --- integrationTests/diagnostics/test.js | 45 +++++ src/execution/Executor.ts | 34 +++- .../__tests__/resolve-diagnostics-test.ts | 173 ++++++++++++++++++ 3 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 src/execution/__tests__/resolve-diagnostics-test.ts diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js index ae25b6ef64..fb67e75d28 100644 --- a/integrationTests/diagnostics/test.js +++ b/integrationTests/diagnostics/test.js @@ -198,6 +198,50 @@ async function runSubscribeCase() { } } +function runResolveCase() { + const schema = buildSchema( + `type Query { hello: String nested: Nested } type Nested { leaf: String }`, + ); + const document = parse('{ hello nested { leaf } }'); + + const events = []; + const handler = { + start: (msg) => + events.push({ + kind: 'start', + fieldName: msg.fieldName, + parentType: msg.parentType, + fieldType: msg.fieldType, + fieldPath: msg.fieldPath, + isTrivialResolver: msg.isTrivialResolver, + }), + end: () => events.push({ kind: 'end' }), + asyncStart: () => events.push({ kind: 'asyncStart' }), + asyncEnd: () => events.push({ kind: 'asyncEnd' }), + error: (msg) => events.push({ kind: 'error', error: msg.error }), + }; + + const channel = dc.tracingChannel('graphql:resolve'); + channel.subscribe(handler); + + try { + const rootValue = { hello: () => 'world', nested: { leaf: 'leaf-value' } }; + execute({ schema, document, rootValue }); + + const starts = events.filter((e) => e.kind === 'start'); + const paths = starts.map((e) => e.fieldPath); + assert.deepEqual(paths, ['hello', 'nested', 'nested.leaf']); + + const hello = starts.find((e) => e.fieldName === 'hello'); + assert.equal(hello.parentType, 'Query'); + assert.equal(hello.fieldType, 'String'); + // buildSchema never attaches field.resolve; all fields report as trivial. + assert.equal(hello.isTrivialResolver, true); + } finally { + channel.unsubscribe(handler); + } +} + function runNoSubscriberCase() { const doc = parse('{ field }'); assert.equal(doc.kind, 'Document'); @@ -208,6 +252,7 @@ async function main() { runValidateCase(); runExecuteCase(); await runSubscribeCase(); + runResolveCase(); runNoSubscriberCase(); console.log('diagnostics integration test passed'); } diff --git a/src/execution/Executor.ts b/src/execution/Executor.ts index 15d88de6b9..7413a2933c 100644 --- a/src/execution/Executor.ts +++ b/src/execution/Executor.ts @@ -44,6 +44,8 @@ import { } from '../type/definition.js'; import type { GraphQLSchema } from '../type/schema.js'; +import { maybeTraceMixed } from '../diagnostics.js'; + import { AbortedGraphQLExecutionError } from './AbortedGraphQLExecutionError.js'; import { withCancellation } from './cancellablePromise.js'; import type { @@ -613,7 +615,11 @@ export class Executor< // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly // used to represent an authenticated user, or request-specific caches. - const result = resolveFn(source, args, contextValue, info); + const result = maybeTraceMixed( + 'resolve', + () => buildResolveCtx(info, args, fieldDef.resolve === undefined), + () => resolveFn(source, args, contextValue, info), + ); if (isPromiseLike(result)) { return this.completePromisedValue( @@ -1396,3 +1402,29 @@ export class Executor< function toNodes(fieldDetailsList: FieldDetailsList): ReadonlyArray { return fieldDetailsList.map((fieldDetails) => fieldDetails.node); } + +/** + * Build a graphql:resolve channel context for a single field invocation. + * + * `fieldPath` is exposed as a lazy getter because serializing the response + * path is O(depth) and APMs that depth-filter or skip trivial resolvers + * often never read it. `args` is passed through by reference. + */ +function buildResolveCtx( + info: GraphQLResolveInfo, + args: { readonly [argument: string]: unknown }, + isTrivialResolver: boolean, +): object { + let cachedFieldPath: string | undefined; + return { + fieldName: info.fieldName, + parentType: info.parentType.name, + fieldType: String(info.returnType), + args, + isTrivialResolver, + get fieldPath() { + cachedFieldPath ??= pathToArray(info.path).join('.'); + return cachedFieldPath; + }, + }; +} diff --git a/src/execution/__tests__/resolve-diagnostics-test.ts b/src/execution/__tests__/resolve-diagnostics-test.ts new file mode 100644 index 0000000000..5d5343e4e1 --- /dev/null +++ b/src/execution/__tests__/resolve-diagnostics-test.ts @@ -0,0 +1,173 @@ +import { expect } from 'chai'; +import { afterEach, beforeEach, describe, it } from 'mocha'; + +import { + collectEvents, + FakeDc, +} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + +import { isPromise } from '../../jsutils/isPromise.js'; + +import { parse } from '../../language/parser.js'; + +import { GraphQLObjectType } from '../../type/definition.js'; +import { GraphQLString } from '../../type/scalars.js'; +import { GraphQLSchema } from '../../type/schema.js'; + +import { buildSchema } from '../../utilities/buildASTSchema.js'; + +import { enableDiagnosticsChannel } from '../../diagnostics.js'; + +import { execute } from '../execute.js'; + +const schema = buildSchema(` + type Query { + sync: String + async: String + fail: String + plain: String + nested: Nested + } + type Nested { + leaf: String + } +`); + +const rootValue = { + sync: () => 'hello', + async: () => Promise.resolve('hello-async'), + fail: () => { + throw new Error('boom'); + }, + // no `plain` resolver, default property-access is used. + plain: 'plain-value', + nested: { leaf: 'leaf-value' }, +}; + +const fakeDc = new FakeDc(); +const resolveChannel = fakeDc.tracingChannel('graphql:resolve'); + +describe('resolve diagnostics channel', () => { + let active: ReturnType | undefined; + + beforeEach(() => { + enableDiagnosticsChannel(fakeDc); + }); + + afterEach(() => { + active?.unsubscribe(); + active = undefined; + }); + + it('emits start and end around a synchronous resolver', () => { + active = collectEvents(resolveChannel); + + const result = execute({ schema, document: parse('{ sync }'), rootValue }); + if (isPromise(result)) { + throw new Error('expected sync'); + } + + const starts = active.events.filter((e) => e.kind === 'start'); + expect(starts.length).to.equal(1); + expect(starts[0].ctx.fieldName).to.equal('sync'); + expect(starts[0].ctx.parentType).to.equal('Query'); + expect(starts[0].ctx.fieldType).to.equal('String'); + expect(starts[0].ctx.fieldPath).to.equal('sync'); + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal(['start', 'end']); + }); + + it('emits the full async lifecycle when a resolver returns a promise', async () => { + active = collectEvents(resolveChannel); + + const result = execute({ schema, document: parse('{ async }'), rootValue }); + await result; + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal(['start', 'end', 'asyncStart', 'asyncEnd']); + }); + + it('emits start, error, end when a sync resolver throws', () => { + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ schema, document: parse('{ fail }'), rootValue }); + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal(['start', 'error', 'end']); + }); + + it('reports isTrivialResolver based on field.resolve presence', () => { + const trivialSchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + trivial: { type: GraphQLString }, + custom: { + type: GraphQLString, + resolve: () => 'explicit', + }, + }, + }), + }); + + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ + schema: trivialSchema, + document: parse('{ trivial custom }'), + rootValue: { trivial: 'value' }, + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + const byField = new Map( + starts.map((e) => [e.ctx.fieldName, e.ctx.isTrivialResolver]), + ); + expect(byField.get('trivial')).to.equal(true); + expect(byField.get('custom')).to.equal(false); + }); + + it('serializes fieldPath lazily, joining path keys with dots', () => { + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ + schema, + document: parse('{ nested { leaf } }'), + rootValue, + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + const paths = starts.map((e) => e.ctx.fieldPath); + expect(paths).to.deep.equal(['nested', 'nested.leaf']); + }); + + it('fires once per field, not per schema walk', () => { + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ + schema, + document: parse('{ sync plain nested { leaf } }'), + rootValue, + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + const endsSync = active.events.filter((e) => e.kind === 'end'); + expect(starts.length).to.equal(4); // sync, plain, nested, nested.leaf + expect(endsSync.length).to.equal(4); + }); + + it('does nothing when no subscribers are attached', () => { + const result = execute({ + schema, + document: parse('{ sync }'), + rootValue, + }); + if (isPromise(result)) { + throw new Error('expected sync'); + } + }); +}); From 8a1e473eb4b214bccce91413df597f9ed7a821fa Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 16:40:21 -0400 Subject: [PATCH 09/29] fix(diagnostics): preserve AsyncLocalStorage across async lifecycle maybeTraceMixed previously delegated start / end to traceSync and then attached its own .then / .finally outside that scope. That worked for event ordering but broke AsyncLocalStorage propagation: a subscriber that called channel.start.bindStore(als, ...) could read that store in its start and end handlers but not in asyncStart or asyncEnd, because those fired in a .then attached outside the start.runStores scope. Promise continuations inherit the AsyncLocalStorage context of the frame that attaches them, so the fix is to attach .then inside the start.runStores block. This mirrors Node's own TracingChannel.tracePromise structure exactly, just with an additional sync branch that short- circuits to [start, end] when fn returns synchronously. Also updates the FakeChannel helper to match: its runStores now publishes the context on entry (the behavior Node's Channel.runStores has), so the fake traceSync / tracePromise implementations match real Node's event counts without the old end.runStores workaround. Adds an integration test that binds a store on graphql:execute's start sub-channel and asserts every lifecycle handler sees it when a resolver returns a promise. --- integrationTests/diagnostics/test.js | 37 +++++++++ src/__testUtils__/fakeDiagnosticsChannel.ts | 83 ++++++++++++--------- src/diagnostics.ts | 65 +++++++++++----- 3 files changed, 131 insertions(+), 54 deletions(-) diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js index fb67e75d28..93ed499b7a 100644 --- a/integrationTests/diagnostics/test.js +++ b/integrationTests/diagnostics/test.js @@ -3,6 +3,7 @@ /* eslint-disable n/no-unsupported-features/node-builtins */ import assert from 'node:assert/strict'; +import { AsyncLocalStorage } from 'node:async_hooks'; import dc from 'node:diagnostics_channel'; import { @@ -247,12 +248,48 @@ function runNoSubscriberCase() { assert.equal(doc.kind, 'Document'); } +async function runAlsPropagationCase() { + // A subscriber that binds a store on the `start` sub-channel should be able + // to read it in every lifecycle handler (start, end, asyncStart, asyncEnd). + // This is what APMs use to parent child spans to the current operation + // without threading state through the ctx object. + const als = new AsyncLocalStorage(); + const channel = dc.tracingChannel('graphql:execute'); + channel.start.bindStore(als, (ctx) => ({ operationName: ctx.operationName })); + + const seen = {}; + const handler = { + start: () => (seen.start = als.getStore()), + end: () => (seen.end = als.getStore()), + asyncStart: () => (seen.asyncStart = als.getStore()), + asyncEnd: () => (seen.asyncEnd = als.getStore()), + }; + channel.subscribe(handler); + + try { + const schema = buildSchema(`type Query { slow: String }`); + const document = parse('query Slow { slow }'); + const rootValue = { slow: () => Promise.resolve('done') }; + + await execute({ schema, document, rootValue }); + + assert.deepEqual(seen.start, { operationName: 'Slow' }); + assert.deepEqual(seen.end, { operationName: 'Slow' }); + assert.deepEqual(seen.asyncStart, { operationName: 'Slow' }); + assert.deepEqual(seen.asyncEnd, { operationName: 'Slow' }); + } finally { + channel.unsubscribe(handler); + channel.start.unbindStore(als); + } +} + async function main() { runParseCases(); runValidateCase(); runExecuteCase(); await runSubscribeCase(); runResolveCase(); + await runAlsPropagationCase(); runNoSubscriberCase(); console.log('diagnostics integration test passed'); } diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts index 4a0bcd7f84..4ec1499046 100644 --- a/src/__testUtils__/fakeDiagnosticsChannel.ts +++ b/src/__testUtils__/fakeDiagnosticsChannel.ts @@ -29,11 +29,15 @@ export class FakeChannel implements MinimalChannel { } runStores( - _ctx: ContextType, + ctx: ContextType, fn: (this: ContextType, ...args: Array) => T, thisArg?: unknown, ...args: Array ): T { + // Node's Channel.runStores publishes the context on the channel before + // invoking fn. Mirror that here so traceSync / tracePromise fake exactly + // matches real Node's start / end event counts. + this.publish(ctx); return fn.apply(thisArg as ContextType, args); } @@ -81,23 +85,24 @@ export class FakeTracingChannel implements MinimalTracingChannel { thisArg?: unknown, ...args: Array ): T { - this.start.publish(ctx); - let result: T; - try { - result = this.end.runStores(ctx, fn, thisArg, ...args); - } catch (err) { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); + return this.start.runStores(ctx, () => { + let result: T; + try { + result = fn.apply(thisArg as object, args); + } catch (err) { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + this.end.publish(ctx); + throw err; + } + // Node's real traceSync sets `ctx.result` before publishing `end`, so + // subscribers can inspect `isPromise(ctx.result)` inside their `end` + // handler to decide whether the operation is complete or async events + // will follow. Match that semantic here. + (ctx as { result: unknown }).result = result; this.end.publish(ctx); - throw err; - } - // Node's real traceSync sets `ctx.result` before publishing `end`, so - // subscribers can inspect `isPromise(ctx.result)` inside their `end` - // handler to decide whether the operation is complete or async events - // will follow. Match that semantic here. - (ctx as { result: unknown }).result = result; - this.end.publish(ctx); - return result; + return result; + }); } tracePromise( @@ -106,33 +111,39 @@ export class FakeTracingChannel implements MinimalTracingChannel { thisArg?: unknown, ...args: Array ): Promise { - this.start.publish(ctx); - let promise: Promise; - try { - promise = this.end.runStores(ctx, fn, thisArg, ...args); - } catch (err) { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); + return this.start.runStores(ctx, () => { + let promise: Promise; + try { + promise = fn.apply(thisArg as object, args); + } catch (err) { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); + this.end.publish(ctx); + throw err; + } this.end.publish(ctx); - throw err; - } - this.end.publish(ctx); - this.asyncStart.publish(ctx); - return promise - .then( + return promise.then( (result) => { (ctx as { result: unknown }).result = result; - return result; + this.asyncStart.publish(ctx); + try { + return result; + } finally { + this.asyncEnd.publish(ctx); + } }, (err: unknown) => { (ctx as { error: unknown }).error = err; this.error.publish(ctx); - throw err; + this.asyncStart.publish(ctx); + try { + throw err; + } finally { + this.asyncEnd.publish(ctx); + } }, - ) - .finally(() => { - this.asyncEnd.publish(ctx); - }); + ); + }); } } diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 4971fc4fd2..b175c8f0c9 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -185,7 +185,21 @@ export function maybeTracePromise( } /** - * Publish a mixed sync-or-promise operation through the named graphql tracing channel. + * Publish a mixed sync-or-promise operation through the named graphql tracing + * channel. + * + * Mirrors Node's own `TracingChannel.tracePromise` for the async branch while + * handling sync returns without the cost of a promise wrap. The entire + * lifecycle runs inside `start.runStores`, which is what lets subscribers + * that call `channel.start.bindStore(als, ...)` read that store in every + * sub-channel handler: promise continuations attached inside a `runStores` + * block inherit the AsyncLocalStorage context via async_hooks, so + * `asyncStart` and `asyncEnd` fire with the same store active as `start` + * and `end`. + * + * Subscribers can inspect `isPromise(ctx.result)` inside their `end` handler + * to know whether `asyncEnd` will follow or the operation is complete. This + * matches Node's convention. * * @internal */ @@ -203,29 +217,44 @@ export function maybeTraceMixed( result?: unknown; }; - // traceSync fires start/end (and error, if fn throws synchronously) - const result = channel.traceSync(fn, ctx); - if (!isPromise(result)) { - return result; - } + return channel.start.runStores(ctx, () => { + let result: T | Promise; + try { + result = fn(); + } catch (err) { + ctx.error = err; + channel.error.publish(ctx); + channel.end.publish(ctx); + throw err; + } - // Fires off `asyncStart` and `asyncEnd` lifecycle events. - channel.asyncStart.publish(ctx); - return result - .then( + if (!isPromise(result)) { + ctx.result = result; + channel.end.publish(ctx); + return result; + } + + channel.end.publish(ctx); + return result.then( (value) => { ctx.result = value; - - return value; + channel.asyncStart.publish(ctx); + try { + return value; + } finally { + channel.asyncEnd.publish(ctx); + } }, (err: unknown) => { ctx.error = err; channel.error.publish(ctx); - - throw err; + channel.asyncStart.publish(ctx); + try { + throw err; + } finally { + channel.asyncEnd.publish(ctx); + } }, - ) - .finally(() => { - channel.asyncEnd.publish(ctx); - }); + ); + }); } From 9efde950877996f2dd06ae9d65eda09a2c6a7235 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 17 Apr 2026 16:44:33 -0400 Subject: [PATCH 10/29] fix(diagnostics): fire asyncStart synchronously, asyncEnd in finally maybeTraceMixed was firing asyncStart and asyncEnd back-to-back inside the .then callback. That left no meaningful window between the two events: for APM subscribers, 'async work begins' and 'async work completes' fired in the same microtask, making the pair useless for measuring the async duration. Correct shape: asyncStart publishes synchronously as soon as we know the operation is in-flight asynchronously (right after end), and asyncEnd publishes in a .finally after settlement. Both the .then and .finally are attached inside start.runStores, so AsyncLocalStorage context is preserved end-to-end via async_hooks. Event order now: sync path: [start, end] async path: [start, end, asyncStart, ..., asyncEnd] async error: [start, end, asyncStart, error, asyncEnd] Mirrors the same fix in the FakeTracingChannel helper. --- src/__testUtils__/fakeDiagnosticsChannel.ts | 33 +++++++++----------- src/diagnostics.ts | 34 +++++++++------------ 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts index 4ec1499046..08c645c2b3 100644 --- a/src/__testUtils__/fakeDiagnosticsChannel.ts +++ b/src/__testUtils__/fakeDiagnosticsChannel.ts @@ -122,27 +122,22 @@ export class FakeTracingChannel implements MinimalTracingChannel { throw err; } this.end.publish(ctx); - return promise.then( - (result) => { - (ctx as { result: unknown }).result = result; - this.asyncStart.publish(ctx); - try { + this.asyncStart.publish(ctx); + return promise + .then( + (result) => { + (ctx as { result: unknown }).result = result; return result; - } finally { - this.asyncEnd.publish(ctx); - } - }, - (err: unknown) => { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - this.asyncStart.publish(ctx); - try { + }, + (err: unknown) => { + (ctx as { error: unknown }).error = err; + this.error.publish(ctx); throw err; - } finally { - this.asyncEnd.publish(ctx); - } - }, - ); + }, + ) + .finally(() => { + this.asyncEnd.publish(ctx); + }); }); } } diff --git a/src/diagnostics.ts b/src/diagnostics.ts index b175c8f0c9..b2fcec7a22 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -235,26 +235,22 @@ export function maybeTraceMixed( } channel.end.publish(ctx); - return result.then( - (value) => { - ctx.result = value; - channel.asyncStart.publish(ctx); - try { + channel.asyncStart.publish(ctx); + + return result + .then( + (value) => { + ctx.result = value; return value; - } finally { - channel.asyncEnd.publish(ctx); - } - }, - (err: unknown) => { - ctx.error = err; - channel.error.publish(ctx); - channel.asyncStart.publish(ctx); - try { + }, + (err: unknown) => { + ctx.error = err; + channel.error.publish(ctx); throw err; - } finally { - channel.asyncEnd.publish(ctx); - } - }, - ); + }, + ) + .finally(() => { + channel.asyncEnd.publish(ctx); + }); }); } From acb19455e9c5aad8b1f758331c36a076e3940341 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Sat, 18 Apr 2026 00:28:54 -0400 Subject: [PATCH 11/29] chore: remove old comments no longer apply --- src/__testUtils__/fakeDiagnosticsChannel.ts | 3 +-- src/diagnostics.ts | 13 ------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts index 08c645c2b3..0a33e24d98 100644 --- a/src/__testUtils__/fakeDiagnosticsChannel.ts +++ b/src/__testUtils__/fakeDiagnosticsChannel.ts @@ -55,8 +55,7 @@ export class FakeChannel implements MinimalChannel { /** * Structurally-faithful `MinimalTracingChannel` implementation mirroring - * Node's `TracingChannel.traceSync` / `tracePromise` lifecycle (start, - * runStores, error, asyncStart, asyncEnd, end). + * Node's `TracingChannel.traceSync` / `tracePromise` lifecycle */ export class FakeTracingChannel implements MinimalTracingChannel { start: FakeChannel = new FakeChannel(); diff --git a/src/diagnostics.ts b/src/diagnostics.ts index b2fcec7a22..4354b6dfc8 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -188,19 +188,6 @@ export function maybeTracePromise( * Publish a mixed sync-or-promise operation through the named graphql tracing * channel. * - * Mirrors Node's own `TracingChannel.tracePromise` for the async branch while - * handling sync returns without the cost of a promise wrap. The entire - * lifecycle runs inside `start.runStores`, which is what lets subscribers - * that call `channel.start.bindStore(als, ...)` read that store in every - * sub-channel handler: promise continuations attached inside a `runStores` - * block inherit the AsyncLocalStorage context via async_hooks, so - * `asyncStart` and `asyncEnd` fire with the same store active as `start` - * and `end`. - * - * Subscribers can inspect `isPromise(ctx.result)` inside their `end` handler - * to know whether `asyncEnd` will follow or the operation is complete. This - * matches Node's convention. - * * @internal */ export function maybeTraceMixed( From ea6e2d52273705951bf45fa21ab20a262ecc1fed Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Sun, 19 Apr 2026 20:42:00 -0400 Subject: [PATCH 12/29] ref: remove tracePromise as it was not needed with runStores --- src/diagnostics.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 4354b6dfc8..c15f4b4b43 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -165,25 +165,6 @@ export function maybeTraceSync( return channel.traceSync(fn, ctxFactory()); } -/** - * Publish a promise-returning operation through the named graphql tracing - * channel, short-circuiting to `fn()` when the channel isn't registered or - * nothing is listening. - * - * @internal - */ -export function maybeTracePromise( - name: keyof GraphQLChannels, - ctxFactory: () => object, - fn: () => Promise, -): Promise { - const channel = getChannels()?.[name]; - if (!shouldTrace(channel)) { - return fn(); - } - return channel.tracePromise(fn, ctxFactory()); -} - /** * Publish a mixed sync-or-promise operation through the named graphql tracing * channel. From 9ad854981d27b787d5a219d384c3f3a031a02abd Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Sun, 19 Apr 2026 21:01:14 -0400 Subject: [PATCH 13/29] ref(perf): cache publish decision in excutor --- src/diagnostics.ts | 2 +- src/execution/Executor.ts | 26 ++++++++++++++++++++------ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/diagnostics.ts b/src/diagnostics.ts index c15f4b4b43..da52dc16fa 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -139,7 +139,7 @@ export function enableDiagnosticsChannel(dc: MinimalDiagnosticsChannel): void { * * @internal */ -function shouldTrace( +export function shouldTrace( channel: MinimalTracingChannel | undefined, ): channel is MinimalTracingChannel { // eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare diff --git a/src/execution/Executor.ts b/src/execution/Executor.ts index 7413a2933c..f277313e42 100644 --- a/src/execution/Executor.ts +++ b/src/execution/Executor.ts @@ -44,7 +44,8 @@ import { } from '../type/definition.js'; import type { GraphQLSchema } from '../type/schema.js'; -import { maybeTraceMixed } from '../diagnostics.js'; +import type { MinimalTracingChannel } from '../diagnostics.js'; +import { getChannels, maybeTraceMixed, shouldTrace } from '../diagnostics.js'; import { AbortedGraphQLExecutionError } from './AbortedGraphQLExecutionError.js'; import { withCancellation } from './cancellablePromise.js'; @@ -245,6 +246,12 @@ export class Executor< values: ReadonlyArray>, ) => Promise>; + // Resolved once per Executor so the per-field gate in `executeField` is a + // single member read + null check, not a `getChannels()?.resolve` walk + + // `hasSubscribers` read on every resolution. Undefined when diagnostics + // are off or nobody is listening at construction time. + _resolveChannel: MinimalTracingChannel | undefined; + constructor( validatedExecutionArgs: ValidatedExecutionArgs, sharedExecutionContext?: SharedExecutionContext, @@ -254,6 +261,11 @@ export class Executor< this.abortReason = defaultAbortReason; this.collectedErrors = new CollectedErrors(); + const resolveChannel = getChannels()?.resolve; + this._resolveChannel = shouldTrace(resolveChannel) + ? resolveChannel + : undefined; + if (sharedExecutionContext === undefined) { this.resolverAbortController = new AbortController(); this.sharedExecutionContext = createSharedExecutionContext( @@ -615,11 +627,13 @@ export class Executor< // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly // used to represent an authenticated user, or request-specific caches. - const result = maybeTraceMixed( - 'resolve', - () => buildResolveCtx(info, args, fieldDef.resolve === undefined), - () => resolveFn(source, args, contextValue, info), - ); + const result = this._resolveChannel + ? maybeTraceMixed( + 'resolve', + () => buildResolveCtx(info, args, fieldDef.resolve === undefined), + () => resolveFn(source, args, contextValue, info), + ) + : resolveFn(source, args, contextValue, info); if (isPromiseLike(result)) { return this.completePromisedValue( From 8a70ca2d93e8aedc4d74a202bcd6a9f6cd952093 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Sun, 19 Apr 2026 21:08:30 -0400 Subject: [PATCH 14/29] test: coverage and cleanup unused type --- src/__testUtils__/fakeDiagnosticsChannel.ts | 45 +++---------------- src/__tests__/diagnostics-test.ts | 3 -- src/diagnostics.ts | 7 --- .../__tests__/resolve-diagnostics-test.ts | 25 +++++++++++ src/index.ts | 2 +- 5 files changed, 31 insertions(+), 51 deletions(-) diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts index 0a33e24d98..6d99986214 100644 --- a/src/__testUtils__/fakeDiagnosticsChannel.ts +++ b/src/__testUtils__/fakeDiagnosticsChannel.ts @@ -14,6 +14,7 @@ export type Listener = (message: unknown) => void; export class FakeChannel implements MinimalChannel { listeners: Array = []; + /* c8 ignore next 3 */ get [Symbol.toStringTag]() { return 'FakeChannel'; } @@ -55,7 +56,7 @@ export class FakeChannel implements MinimalChannel { /** * Structurally-faithful `MinimalTracingChannel` implementation mirroring - * Node's `TracingChannel.traceSync` / `tracePromise` lifecycle + * Node's `TracingChannel.traceSync` lifecycle. */ export class FakeTracingChannel implements MinimalTracingChannel { start: FakeChannel = new FakeChannel(); @@ -64,6 +65,7 @@ export class FakeTracingChannel implements MinimalTracingChannel { asyncEnd: FakeChannel = new FakeChannel(); error: FakeChannel = new FakeChannel(); + /* c8 ignore next 3 */ get [Symbol.toStringTag]() { return 'FakeTracingChannel'; } @@ -95,55 +97,18 @@ export class FakeTracingChannel implements MinimalTracingChannel { throw err; } // Node's real traceSync sets `ctx.result` before publishing `end`, so - // subscribers can inspect `isPromise(ctx.result)` inside their `end` - // handler to decide whether the operation is complete or async events - // will follow. Match that semantic here. + // subscribers can inspect `ctx.result` inside their `end` handler. (ctx as { result: unknown }).result = result; this.end.publish(ctx); return result; }); } - - tracePromise( - fn: (...args: Array) => Promise, - ctx: object, - thisArg?: unknown, - ...args: Array - ): Promise { - return this.start.runStores(ctx, () => { - let promise: Promise; - try { - promise = fn.apply(thisArg as object, args); - } catch (err) { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - this.end.publish(ctx); - throw err; - } - this.end.publish(ctx); - this.asyncStart.publish(ctx); - return promise - .then( - (result) => { - (ctx as { result: unknown }).result = result; - return result; - }, - (err: unknown) => { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - throw err; - }, - ) - .finally(() => { - this.asyncEnd.publish(ctx); - }); - }); - } } export class FakeDc implements MinimalDiagnosticsChannel { private cache = new Map(); + /* c8 ignore next 3 */ get [Symbol.toStringTag]() { return 'FakeDc'; } diff --git a/src/__tests__/diagnostics-test.ts b/src/__tests__/diagnostics-test.ts index e620c86fd7..06a0c5ff3a 100644 --- a/src/__tests__/diagnostics-test.ts +++ b/src/__tests__/diagnostics-test.ts @@ -29,9 +29,6 @@ function fakeTracingChannel(name: string): MinimalTracingChannel { asyncEnd: noop, error: noop, traceSync: (fn: (...args: Array) => T): T => fn(), - tracePromise: ( - fn: (...args: Array) => Promise, - ): Promise => fn(), }; return channel; } diff --git a/src/diagnostics.ts b/src/diagnostics.ts index da52dc16fa..52d65688d3 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -49,13 +49,6 @@ export interface MinimalTracingChannel { thisArg?: unknown, ...args: Array ) => T; - - tracePromise: ( - fn: (...args: Array) => Promise, - ctx: object, - thisArg?: unknown, - ...args: Array - ) => Promise; } /** diff --git a/src/execution/__tests__/resolve-diagnostics-test.ts b/src/execution/__tests__/resolve-diagnostics-test.ts index 5d5343e4e1..1608bc4471 100644 --- a/src/execution/__tests__/resolve-diagnostics-test.ts +++ b/src/execution/__tests__/resolve-diagnostics-test.ts @@ -25,6 +25,7 @@ const schema = buildSchema(` sync: String async: String fail: String + asyncFail: String plain: String nested: Nested } @@ -39,6 +40,7 @@ const rootValue = { fail: () => { throw new Error('boom'); }, + asyncFail: () => Promise.reject(new Error('async-boom')), // no `plain` resolver, default property-access is used. plain: 'plain-value', nested: { leaf: 'leaf-value' }, @@ -98,6 +100,29 @@ describe('resolve diagnostics channel', () => { expect(kinds).to.deep.equal(['start', 'error', 'end']); }); + it('emits full async lifecycle with error when a resolver rejects', async () => { + active = collectEvents(resolveChannel); + + await execute({ + schema, + document: parse('{ asyncFail }'), + rootValue, + }); + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal([ + 'start', + 'end', + 'asyncStart', + 'error', + 'asyncEnd', + ]); + const errorEvent = active.events.find((e) => e.kind === 'error'); + expect((errorEvent?.ctx as { error?: Error }).error?.message).to.equal( + 'async-boom', + ); + }); + it('reports isTrivialResolver based on field.resolve presence', () => { const trivialSchema = new GraphQLSchema({ query: new GraphQLObjectType({ diff --git a/src/index.ts b/src/index.ts index f714d2ea60..ec1001325e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,7 +34,7 @@ export { enableDevMode, isDevModeEnabled } from './devMode.js'; // Register a `node:diagnostics_channel`-compatible module to enable // tracing channel emission from parse, validate, execute, subscribe, -// and resolver lifecycles. +// and resolver lifecycle events. export { enableDiagnosticsChannel } from './diagnostics.js'; export type { MinimalChannel, From 11e890dce563aba46d2ccf160423a6be4eb61871 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Tue, 21 Apr 2026 11:48:05 -0400 Subject: [PATCH 15/29] feat(diagnostics): throw on re-registration with different dc module Calling enableDiagnosticsChannel again with the same dc is now a no-op; a different dc throws to surface the misconfiguration rather than silently replacing channel references that subscribers already hold. --- src/__testUtils__/fakeDiagnosticsChannel.ts | 8 ++ src/__tests__/diagnostics-test.ts | 117 +++++------------- src/diagnostics.ts | 22 +++- .../__tests__/execute-diagnostics-test.ts | 7 +- .../__tests__/resolve-diagnostics-test.ts | 7 +- .../__tests__/subscribe-diagnostics-test.ts | 7 +- .../__tests__/parser-diagnostics-test.ts | 7 +- .../__tests__/validate-diagnostics-test.ts | 7 +- 8 files changed, 72 insertions(+), 110 deletions(-) diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts index 6d99986214..3c6b3821fc 100644 --- a/src/__testUtils__/fakeDiagnosticsChannel.ts +++ b/src/__testUtils__/fakeDiagnosticsChannel.ts @@ -123,6 +123,14 @@ export class FakeDc implements MinimalDiagnosticsChannel { } } +/** + * Shared fake `diagnostics_channel` instance used across all diagnostics + * test suites. `enableDiagnosticsChannel` now throws when called with a + * different `dc` module than the one previously registered, so all test + * files must register the same instance. + */ +export const sharedFakeDc: FakeDc = new FakeDc(); + export interface CollectedEvent { kind: 'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error'; ctx: { [key: string]: unknown }; diff --git a/src/__tests__/diagnostics-test.ts b/src/__tests__/diagnostics-test.ts index 06a0c5ff3a..a87391d020 100644 --- a/src/__tests__/diagnostics-test.ts +++ b/src/__tests__/diagnostics-test.ts @@ -1,108 +1,55 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; +import { sharedFakeDc } from '../__testUtils__/fakeDiagnosticsChannel.js'; + import { invariant } from '../jsutils/invariant.js'; -import type { - MinimalDiagnosticsChannel, - MinimalTracingChannel, -} from '../diagnostics.js'; import { enableDiagnosticsChannel, getChannels } from '../diagnostics.js'; -function fakeTracingChannel(name: string): MinimalTracingChannel { - const noop: MinimalTracingChannel['start'] = { - hasSubscribers: false, - publish: () => { - /* noop */ - }, - runStores: ( - _ctx: unknown, - fn: (this: unknown, ...args: Array) => T, - ): T => fn(), - }; - const channel: MinimalTracingChannel & { _name: string } = { - _name: name, - hasSubscribers: false, - start: noop, - end: noop, - asyncStart: noop, - asyncEnd: noop, - error: noop, - traceSync: (fn: (...args: Array) => T): T => fn(), - }; - return channel; -} - -function fakeDc(): MinimalDiagnosticsChannel & { - created: Array; -} { - const created: Array = []; - const cache = new Map(); - return { - created, - tracingChannel(name: string) { - let existing = cache.get(name); - if (existing === undefined) { - created.push(name); - existing = fakeTracingChannel(name); - cache.set(name, existing); - } - return existing; - }, - }; -} - describe('diagnostics', () => { - it('registers the five graphql tracing channels', () => { - const dc = fakeDc(); - enableDiagnosticsChannel(dc); - - expect(dc.created).to.deep.equal([ - 'graphql:execute', - 'graphql:parse', - 'graphql:validate', - 'graphql:resolve', - 'graphql:subscribe', - ]); + it('exposes the five graphql tracing channels after registration', () => { + enableDiagnosticsChannel(sharedFakeDc); const channels = getChannels(); invariant(channels !== undefined); - expect(channels.execute).to.not.equal(undefined); - expect(channels.parse).to.not.equal(undefined); - expect(channels.validate).to.not.equal(undefined); - expect(channels.resolve).to.not.equal(undefined); - expect(channels.subscribe).to.not.equal(undefined); + expect(channels.execute).to.equal( + sharedFakeDc.tracingChannel('graphql:execute'), + ); + expect(channels.parse).to.equal( + sharedFakeDc.tracingChannel('graphql:parse'), + ); + expect(channels.validate).to.equal( + sharedFakeDc.tracingChannel('graphql:validate'), + ); + expect(channels.resolve).to.equal( + sharedFakeDc.tracingChannel('graphql:resolve'), + ); + expect(channels.subscribe).to.equal( + sharedFakeDc.tracingChannel('graphql:subscribe'), + ); }); - it('re-registration with the same module preserves channel identity', () => { - const dc = fakeDc(); - enableDiagnosticsChannel(dc); + it('re-registration with the same module is a no-op', () => { + enableDiagnosticsChannel(sharedFakeDc); const first = getChannels(); invariant(first !== undefined); - enableDiagnosticsChannel(dc); + enableDiagnosticsChannel(sharedFakeDc); const second = getChannels(); - invariant(second !== undefined); - expect(second.execute).to.equal(first.execute); - expect(second.parse).to.equal(first.parse); - expect(second.validate).to.equal(first.validate); - expect(second.resolve).to.equal(first.resolve); - expect(second.subscribe).to.equal(first.subscribe); + expect(second).to.equal(first); }); - it('re-registration with a different module replaces stored references', () => { - const dc1 = fakeDc(); - const dc2 = fakeDc(); - - enableDiagnosticsChannel(dc1); - const first = getChannels(); - invariant(first !== undefined); - - enableDiagnosticsChannel(dc2); - const second = getChannels(); - invariant(second !== undefined); + it('re-registration with a different module throws', () => { + enableDiagnosticsChannel(sharedFakeDc); - expect(second.execute).to.not.equal(first.execute); + expect(() => + enableDiagnosticsChannel({ + tracingChannel: () => { + throw new Error('should not be called'); + }, + }), + ).to.throw(/different `diagnostics_channel` module/); }); }); diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 52d65688d3..33208d194d 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -74,6 +74,7 @@ export interface GraphQLChannels { } let channels: GraphQLChannels | undefined; +let registeredDc: MinimalDiagnosticsChannel | undefined; /** * Internal accessor used at emission sites. Returns `undefined` when no @@ -98,20 +99,31 @@ export function getChannels(): GraphQLChannels | undefined { * - `graphql:subscribe` * - `graphql:resolve` * - * Calling this repeatedly is safe: subsequent calls replace the stored - * channel references, but since `tracingChannel(name)` is cached by name, - * the channel identities remain stable across registrations from the same - * underlying module. + * @throws {Error} If a different `diagnostics_channel` module is registered. * * @example * ```ts * import dc from 'node:diagnostics_channel'; * import { enableDiagnosticsChannel } from 'graphql'; * - * enableDiagnosticsChannel(dc); + * try { + * enableDiagnosticsChannel(dc); + * } catch { + * // A diagnostic_channel module was already registered, safe to subscribe. + * } * ``` */ export function enableDiagnosticsChannel(dc: MinimalDiagnosticsChannel): void { + if (registeredDc !== undefined) { + if (registeredDc !== dc) { + throw new Error( + 'enableDiagnosticsChannel was called with a different `diagnostics_channel` module than the one previously registered. graphql-js can only publish to one module at a time; ensure all APMs share the same `node:diagnostics_channel` import.', + ); + } + return; + } + + registeredDc = dc; channels = { execute: dc.tracingChannel('graphql:execute'), parse: dc.tracingChannel('graphql:parse'), diff --git a/src/execution/__tests__/execute-diagnostics-test.ts b/src/execution/__tests__/execute-diagnostics-test.ts index 45fcbdee3c..e8ecea8266 100644 --- a/src/execution/__tests__/execute-diagnostics-test.ts +++ b/src/execution/__tests__/execute-diagnostics-test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha'; import { collectEvents, - FakeDc, + sharedFakeDc, } from '../../__testUtils__/fakeDiagnosticsChannel.js'; import { parse } from '../../language/parser.js'; @@ -32,14 +32,13 @@ const rootValue = { async: () => Promise.resolve('hello-async'), }; -const fakeDc = new FakeDc(); -const executeChannel = fakeDc.tracingChannel('graphql:execute'); +const executeChannel = sharedFakeDc.tracingChannel('graphql:execute'); describe('execute diagnostics channel', () => { let active: ReturnType | undefined; beforeEach(() => { - enableDiagnosticsChannel(fakeDc); + enableDiagnosticsChannel(sharedFakeDc); }); afterEach(() => { diff --git a/src/execution/__tests__/resolve-diagnostics-test.ts b/src/execution/__tests__/resolve-diagnostics-test.ts index 1608bc4471..c524a2c8f5 100644 --- a/src/execution/__tests__/resolve-diagnostics-test.ts +++ b/src/execution/__tests__/resolve-diagnostics-test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha'; import { collectEvents, - FakeDc, + sharedFakeDc, } from '../../__testUtils__/fakeDiagnosticsChannel.js'; import { isPromise } from '../../jsutils/isPromise.js'; @@ -46,14 +46,13 @@ const rootValue = { nested: { leaf: 'leaf-value' }, }; -const fakeDc = new FakeDc(); -const resolveChannel = fakeDc.tracingChannel('graphql:resolve'); +const resolveChannel = sharedFakeDc.tracingChannel('graphql:resolve'); describe('resolve diagnostics channel', () => { let active: ReturnType | undefined; beforeEach(() => { - enableDiagnosticsChannel(fakeDc); + enableDiagnosticsChannel(sharedFakeDc); }); afterEach(() => { diff --git a/src/execution/__tests__/subscribe-diagnostics-test.ts b/src/execution/__tests__/subscribe-diagnostics-test.ts index a1b1987ecb..99c497a613 100644 --- a/src/execution/__tests__/subscribe-diagnostics-test.ts +++ b/src/execution/__tests__/subscribe-diagnostics-test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha'; import { collectEvents, - FakeDc, + sharedFakeDc, } from '../../__testUtils__/fakeDiagnosticsChannel.js'; import { isPromise } from '../../jsutils/isPromise.js'; @@ -44,14 +44,13 @@ async function* twoTicks(): AsyncIterable<{ tick: string }> { yield { tick: 'two' }; } -const fakeDc = new FakeDc(); -const subscribeChannel = fakeDc.tracingChannel('graphql:subscribe'); +const subscribeChannel = sharedFakeDc.tracingChannel('graphql:subscribe'); describe('subscribe diagnostics channel', () => { let active: ReturnType | undefined; beforeEach(() => { - enableDiagnosticsChannel(fakeDc); + enableDiagnosticsChannel(sharedFakeDc); }); afterEach(() => { diff --git a/src/language/__tests__/parser-diagnostics-test.ts b/src/language/__tests__/parser-diagnostics-test.ts index a970a73508..9ddfdf30d9 100644 --- a/src/language/__tests__/parser-diagnostics-test.ts +++ b/src/language/__tests__/parser-diagnostics-test.ts @@ -3,21 +3,20 @@ import { afterEach, beforeEach, describe, it } from 'mocha'; import { collectEvents, - FakeDc, + sharedFakeDc, } from '../../__testUtils__/fakeDiagnosticsChannel.js'; import { enableDiagnosticsChannel } from '../../diagnostics.js'; import { parse } from '../parser.js'; -const fakeDc = new FakeDc(); -const parseChannel = fakeDc.tracingChannel('graphql:parse'); +const parseChannel = sharedFakeDc.tracingChannel('graphql:parse'); describe('parse diagnostics channel', () => { let active: ReturnType | undefined; beforeEach(() => { - enableDiagnosticsChannel(fakeDc); + enableDiagnosticsChannel(sharedFakeDc); }); afterEach(() => { diff --git a/src/validation/__tests__/validate-diagnostics-test.ts b/src/validation/__tests__/validate-diagnostics-test.ts index 28d1c40c3b..622d44c5f8 100644 --- a/src/validation/__tests__/validate-diagnostics-test.ts +++ b/src/validation/__tests__/validate-diagnostics-test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha'; import { collectEvents, - FakeDc, + sharedFakeDc, } from '../../__testUtils__/fakeDiagnosticsChannel.js'; import { parse } from '../../language/parser.js'; @@ -20,14 +20,13 @@ const schema = buildSchema(` } `); -const fakeDc = new FakeDc(); -const validateChannel = fakeDc.tracingChannel('graphql:validate'); +const validateChannel = sharedFakeDc.tracingChannel('graphql:validate'); describe('validate diagnostics channel', () => { let active: ReturnType | undefined; beforeEach(() => { - enableDiagnosticsChannel(fakeDc); + enableDiagnosticsChannel(sharedFakeDc); }); afterEach(() => { From b84efbd9588291bbb8754e2021c8c106730c786e Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Tue, 21 Apr 2026 13:34:15 -0400 Subject: [PATCH 16/29] feat(diagnostics): allow re-registration with equivalent tracingChannel function --- src/__tests__/diagnostics-test.ts | 16 ++++++++++++++++ src/diagnostics.ts | 8 +++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/__tests__/diagnostics-test.ts b/src/__tests__/diagnostics-test.ts index a87391d020..fa538207fe 100644 --- a/src/__tests__/diagnostics-test.ts +++ b/src/__tests__/diagnostics-test.ts @@ -52,4 +52,20 @@ describe('diagnostics', () => { }), ).to.throw(/different `diagnostics_channel` module/); }); + + it('re-registration accepts a wrapper sharing the same tracingChannel fn', () => { + enableDiagnosticsChannel(sharedFakeDc); + const first = getChannels(); + invariant(first !== undefined); + + // Models an ESM Module Namespace object: distinct outer reference, but + // the `tracingChannel` property is strictly the same function as the + // default export's, so it routes to the same underlying channel cache. + const namespaceLike = { tracingChannel: sharedFakeDc.tracingChannel }; + expect(namespaceLike).to.not.equal(sharedFakeDc); + expect(namespaceLike.tracingChannel).to.equal(sharedFakeDc.tracingChannel); + + expect(() => enableDiagnosticsChannel(namespaceLike)).to.not.throw(); + expect(getChannels()).to.equal(first); + }); }); diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 33208d194d..fd61212084 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -99,6 +99,8 @@ export function getChannels(): GraphQLChannels | undefined { * - `graphql:subscribe` * - `graphql:resolve` * + * Re-registration is tolerated when the incoming `dc` exposes the same + * `tracingChannel` function as the previously registered one * @throws {Error} If a different `diagnostics_channel` module is registered. * * @example @@ -115,7 +117,11 @@ export function getChannels(): GraphQLChannels | undefined { */ export function enableDiagnosticsChannel(dc: MinimalDiagnosticsChannel): void { if (registeredDc !== undefined) { - if (registeredDc !== dc) { + // Compare `tracingChannel` function identity rather than module identity + // so consumers that pass an ESM Module Namespace object and consumers + // that pass the default export of the same underlying module are + // treated as equivalent + if (registeredDc.tracingChannel !== dc.tracingChannel) { throw new Error( 'enableDiagnosticsChannel was called with a different `diagnostics_channel` module than the one previously registered. graphql-js can only publish to one module at a time; ensure all APMs share the same `node:diagnostics_channel` import.', ); From 2a9d88a33320b36d67900c04e1e1039baf5388e8 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 24 Apr 2026 02:49:39 -0400 Subject: [PATCH 17/29] ref: autoload tracing channels --- integrationTests/diagnostics/test.js | 11 +- src/__testUtils__/diagnosticsTestUtils.ts | 54 ++++++ src/__testUtils__/fakeDiagnosticsChannel.ts | 172 ------------------ src/__tests__/diagnostics-test.ts | 78 ++------ src/diagnostics.ts | 124 +++++-------- .../__tests__/execute-diagnostics-test.ts | 14 +- .../__tests__/resolve-diagnostics-test.ts | 14 +- .../__tests__/subscribe-diagnostics-test.ts | 14 +- src/index.ts | 14 +- .../__tests__/parser-diagnostics-test.ts | 14 +- .../__tests__/validate-diagnostics-test.ts | 14 +- 11 files changed, 148 insertions(+), 375 deletions(-) create mode 100644 src/__testUtils__/diagnosticsTestUtils.ts delete mode 100644 src/__testUtils__/fakeDiagnosticsChannel.ts diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js index 93ed499b7a..8a0e5cebb4 100644 --- a/integrationTests/diagnostics/test.js +++ b/integrationTests/diagnostics/test.js @@ -6,16 +6,7 @@ import assert from 'node:assert/strict'; import { AsyncLocalStorage } from 'node:async_hooks'; import dc from 'node:diagnostics_channel'; -import { - buildSchema, - enableDiagnosticsChannel, - execute, - parse, - subscribe, - validate, -} from 'graphql'; - -enableDiagnosticsChannel(dc); +import { buildSchema, execute, parse, subscribe, validate } from 'graphql'; function runParseCases() { // graphql:parse - synchronous. diff --git a/src/__testUtils__/diagnosticsTestUtils.ts b/src/__testUtils__/diagnosticsTestUtils.ts new file mode 100644 index 0000000000..e7b92edab9 --- /dev/null +++ b/src/__testUtils__/diagnosticsTestUtils.ts @@ -0,0 +1,54 @@ +/* eslint-disable n/no-unsupported-features/node-builtins, import/no-nodejs-modules */ +import dc from 'node:diagnostics_channel'; + +import type { MinimalTracingChannel } from '../diagnostics.js'; + +export interface CollectedEvent { + kind: 'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error'; + ctx: { [key: string]: unknown }; +} + +/** + * Subscribe to every lifecycle sub-channel on a TracingChannel and collect + * events in order. Returns the event buffer plus an unsubscribe hook. + */ +export function collectEvents(channel: MinimalTracingChannel): { + events: Array; + unsubscribe: () => void; +} { + const events: Array = []; + const handler = { + start: (ctx: unknown) => + events.push({ kind: 'start', ctx: ctx as { [key: string]: unknown } }), + end: (ctx: unknown) => + events.push({ kind: 'end', ctx: ctx as { [key: string]: unknown } }), + asyncStart: (ctx: unknown) => + events.push({ + kind: 'asyncStart', + ctx: ctx as { [key: string]: unknown }, + }), + asyncEnd: (ctx: unknown) => + events.push({ + kind: 'asyncEnd', + ctx: ctx as { [key: string]: unknown }, + }), + error: (ctx: unknown) => + events.push({ kind: 'error', ctx: ctx as { [key: string]: unknown } }), + }; + (channel as unknown as dc.TracingChannel).subscribe(handler); + return { + events, + unsubscribe() { + (channel as unknown as dc.TracingChannel).unsubscribe(handler); + }, + }; +} + +/** + * Resolve a graphql tracing channel by name on the real + * `node:diagnostics_channel`. graphql-js publishes on the same channels at + * module load. + */ +export function getTracingChannel(name: string): MinimalTracingChannel { + return dc.tracingChannel(name) as unknown as MinimalTracingChannel; +} diff --git a/src/__testUtils__/fakeDiagnosticsChannel.ts b/src/__testUtils__/fakeDiagnosticsChannel.ts deleted file mode 100644 index 3c6b3821fc..0000000000 --- a/src/__testUtils__/fakeDiagnosticsChannel.ts +++ /dev/null @@ -1,172 +0,0 @@ -import type { - MinimalChannel, - MinimalDiagnosticsChannel, - MinimalTracingChannel, -} from '../diagnostics.js'; - -export type Listener = (message: unknown) => void; - -/** - * In-memory `MinimalChannel` implementation used by the unit tests. Tracks - * subscribers and replays Node's `runStores` semantics by simply invoking - * `fn`. - */ -export class FakeChannel implements MinimalChannel { - listeners: Array = []; - - /* c8 ignore next 3 */ - get [Symbol.toStringTag]() { - return 'FakeChannel'; - } - - get hasSubscribers(): boolean { - return this.listeners.length > 0; - } - - publish(message: unknown): void { - for (const l of this.listeners) { - l(message); - } - } - - runStores( - ctx: ContextType, - fn: (this: ContextType, ...args: Array) => T, - thisArg?: unknown, - ...args: Array - ): T { - // Node's Channel.runStores publishes the context on the channel before - // invoking fn. Mirror that here so traceSync / tracePromise fake exactly - // matches real Node's start / end event counts. - this.publish(ctx); - return fn.apply(thisArg as ContextType, args); - } - - subscribe(listener: Listener): void { - this.listeners.push(listener); - } - - unsubscribe(listener: Listener): void { - const idx = this.listeners.indexOf(listener); - if (idx >= 0) { - this.listeners.splice(idx, 1); - } - } -} - -/** - * Structurally-faithful `MinimalTracingChannel` implementation mirroring - * Node's `TracingChannel.traceSync` lifecycle. - */ -export class FakeTracingChannel implements MinimalTracingChannel { - start: FakeChannel = new FakeChannel(); - end: FakeChannel = new FakeChannel(); - asyncStart: FakeChannel = new FakeChannel(); - asyncEnd: FakeChannel = new FakeChannel(); - error: FakeChannel = new FakeChannel(); - - /* c8 ignore next 3 */ - get [Symbol.toStringTag]() { - return 'FakeTracingChannel'; - } - - get hasSubscribers(): boolean { - return ( - this.start.hasSubscribers || - this.end.hasSubscribers || - this.asyncStart.hasSubscribers || - this.asyncEnd.hasSubscribers || - this.error.hasSubscribers - ); - } - - traceSync( - fn: (...args: Array) => T, - ctx: object, - thisArg?: unknown, - ...args: Array - ): T { - return this.start.runStores(ctx, () => { - let result: T; - try { - result = fn.apply(thisArg as object, args); - } catch (err) { - (ctx as { error: unknown }).error = err; - this.error.publish(ctx); - this.end.publish(ctx); - throw err; - } - // Node's real traceSync sets `ctx.result` before publishing `end`, so - // subscribers can inspect `ctx.result` inside their `end` handler. - (ctx as { result: unknown }).result = result; - this.end.publish(ctx); - return result; - }); - } -} - -export class FakeDc implements MinimalDiagnosticsChannel { - private cache = new Map(); - - /* c8 ignore next 3 */ - get [Symbol.toStringTag]() { - return 'FakeDc'; - } - - tracingChannel(name: string): FakeTracingChannel { - let existing = this.cache.get(name); - if (existing === undefined) { - existing = new FakeTracingChannel(); - this.cache.set(name, existing); - } - return existing; - } -} - -/** - * Shared fake `diagnostics_channel` instance used across all diagnostics - * test suites. `enableDiagnosticsChannel` now throws when called with a - * different `dc` module than the one previously registered, so all test - * files must register the same instance. - */ -export const sharedFakeDc: FakeDc = new FakeDc(); - -export interface CollectedEvent { - kind: 'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error'; - ctx: { [key: string]: unknown }; -} - -/** - * Attach listeners to every sub-channel on a FakeTracingChannel and return - * the captured event buffer plus an unsubscribe hook. - */ -export function collectEvents(channel: FakeTracingChannel): { - events: Array; - unsubscribe: () => void; -} { - const events: Array = []; - const make = - (kind: CollectedEvent['kind']): Listener => - (m) => - events.push({ kind, ctx: m as { [key: string]: unknown } }); - const startL = make('start'); - const endL = make('end'); - const asyncStartL = make('asyncStart'); - const asyncEndL = make('asyncEnd'); - const errorL = make('error'); - channel.start.subscribe(startL); - channel.end.subscribe(endL); - channel.asyncStart.subscribe(asyncStartL); - channel.asyncEnd.subscribe(asyncEndL); - channel.error.subscribe(errorL); - return { - events, - unsubscribe() { - channel.start.unsubscribe(startL); - channel.end.unsubscribe(endL); - channel.asyncStart.unsubscribe(asyncStartL); - channel.asyncEnd.unsubscribe(asyncEndL); - channel.error.unsubscribe(errorL); - }, - }; -} diff --git a/src/__tests__/diagnostics-test.ts b/src/__tests__/diagnostics-test.ts index fa538207fe..9c62ce9b9c 100644 --- a/src/__tests__/diagnostics-test.ts +++ b/src/__tests__/diagnostics-test.ts @@ -1,71 +1,31 @@ +/* eslint-disable import/no-nodejs-modules */ +import dc from 'node:diagnostics_channel'; + import { expect } from 'chai'; import { describe, it } from 'mocha'; -import { sharedFakeDc } from '../__testUtils__/fakeDiagnosticsChannel.js'; - import { invariant } from '../jsutils/invariant.js'; -import { enableDiagnosticsChannel, getChannels } from '../diagnostics.js'; +import { getChannels } from '../diagnostics.js'; describe('diagnostics', () => { - it('exposes the five graphql tracing channels after registration', () => { - enableDiagnosticsChannel(sharedFakeDc); - + it('auto-registers the five graphql tracing channels', () => { const channels = getChannels(); invariant(channels !== undefined); - expect(channels.execute).to.equal( - sharedFakeDc.tracingChannel('graphql:execute'), - ); - expect(channels.parse).to.equal( - sharedFakeDc.tracingChannel('graphql:parse'), - ); - expect(channels.validate).to.equal( - sharedFakeDc.tracingChannel('graphql:validate'), - ); - expect(channels.resolve).to.equal( - sharedFakeDc.tracingChannel('graphql:resolve'), - ); - expect(channels.subscribe).to.equal( - sharedFakeDc.tracingChannel('graphql:subscribe'), - ); - }); - - it('re-registration with the same module is a no-op', () => { - enableDiagnosticsChannel(sharedFakeDc); - const first = getChannels(); - invariant(first !== undefined); - - enableDiagnosticsChannel(sharedFakeDc); - const second = getChannels(); - - expect(second).to.equal(first); - }); - - it('re-registration with a different module throws', () => { - enableDiagnosticsChannel(sharedFakeDc); - - expect(() => - enableDiagnosticsChannel({ - tracingChannel: () => { - throw new Error('should not be called'); - }, - }), - ).to.throw(/different `diagnostics_channel` module/); - }); - - it('re-registration accepts a wrapper sharing the same tracingChannel fn', () => { - enableDiagnosticsChannel(sharedFakeDc); - const first = getChannels(); - invariant(first !== undefined); - - // Models an ESM Module Namespace object: distinct outer reference, but - // the `tracingChannel` property is strictly the same function as the - // default export's, so it routes to the same underlying channel cache. - const namespaceLike = { tracingChannel: sharedFakeDc.tracingChannel }; - expect(namespaceLike).to.not.equal(sharedFakeDc); - expect(namespaceLike.tracingChannel).to.equal(sharedFakeDc.tracingChannel); - expect(() => enableDiagnosticsChannel(namespaceLike)).to.not.throw(); - expect(getChannels()).to.equal(first); + // Node's `tracingChannel(name)` returns a fresh wrapper per call but + // the underlying sub-channels are cached by name, so compare those. + const byName = { + execute: 'graphql:execute', + parse: 'graphql:parse', + validate: 'graphql:validate', + resolve: 'graphql:resolve', + subscribe: 'graphql:subscribe', + } as const; + for (const [key, name] of Object.entries(byName)) { + expect(channels[key as keyof typeof byName].start).to.equal( + dc.channel(`tracing:${name}:start`), + ); + } }); }); diff --git a/src/diagnostics.ts b/src/diagnostics.ts index fd61212084..9e731885c8 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -1,16 +1,14 @@ +/* eslint-disable no-undef, import/no-nodejs-modules, n/global-require, @typescript-eslint/no-require-imports */ /** * TracingChannel integration. * - * graphql-js exposes a set of named tracing channels that APM tools can - * subscribe to in order to observe parse, validate, execute, subscribe, and - * resolver lifecycle events. To preserve the isomorphic invariant of the - * core (no runtime-specific imports in `src/`), graphql-js does not import - * `node:diagnostics_channel` itself. Instead, APMs (or runtime-specific - * adapters) hand in a module satisfying `MinimalDiagnosticsChannel` via - * `enableDiagnosticsChannel`. - * - * Channel names are owned by graphql-js so multiple APMs converge on the - * same `TracingChannel` instances and all subscribers coexist. + * graphql-js publishes lifecycle events on a set of named tracing channels + * that APM tools can subscribe to in order to observe parse, validate, + * execute, subscribe, and resolver behavior. At module load time graphql-js + * resolves `node:diagnostics_channel` itself so APMs do not need to interact + * with the graphql API to enable tracing. On runtimes that do not expose + * `node:diagnostics_channel` (e.g., browsers) the load silently no-ops and + * emission sites short-circuit. */ import { isPromise } from './jsutils/isPromise.js'; @@ -18,6 +16,8 @@ import { isPromise } from './jsutils/isPromise.js'; /** * Structural subset of `DiagnosticsChannel` sufficient for publishing and * subscriber gating. `node:diagnostics_channel`'s `Channel` satisfies this. + * + * @internal */ export interface MinimalChannel { readonly hasSubscribers: boolean; @@ -34,6 +34,8 @@ export interface MinimalChannel { * Structural subset of Node's `TracingChannel`. The `node:diagnostics_channel` * `TracingChannel` satisfies this by duck typing, so graphql-js does not need * a dependency on `@types/node` or on the runtime itself. + * + * @internal */ export interface MinimalTracingChannel { readonly hasSubscribers: boolean; @@ -51,11 +53,7 @@ export interface MinimalTracingChannel { ) => T; } -/** - * Structural subset of `node:diagnostics_channel` covering just what - * graphql-js needs at registration time. - */ -export interface MinimalDiagnosticsChannel { +interface DiagnosticsChannelModule { tracingChannel: (name: string) => MinimalTracingChannel; } @@ -73,13 +71,44 @@ export interface GraphQLChannels { subscribe: MinimalTracingChannel; } -let channels: GraphQLChannels | undefined; -let registeredDc: MinimalDiagnosticsChannel | undefined; +function resolveDiagnosticsChannel(): DiagnosticsChannelModule | undefined { + let dc: DiagnosticsChannelModule | undefined; + try { + if ( + typeof process !== 'undefined' && + typeof (process as { getBuiltinModule?: (id: string) => unknown }) + .getBuiltinModule === 'function' + ) { + dc = ( + process as { getBuiltinModule: (id: string) => DiagnosticsChannelModule } + ).getBuiltinModule('node:diagnostics_channel'); + } + if (!dc && typeof require === 'function') { + // CJS fallback for runtimes that lack `process.getBuiltinModule` + // (e.g. Node 20.0 - 20.15). ESM builds skip this branch because + // `require` is undeclared there. + dc = require('node:diagnostics_channel') as DiagnosticsChannelModule; + } + } catch { + // diagnostics_channel not available on this runtime; tracing is a no-op. + } + return dc; +} + +const dc = resolveDiagnosticsChannel(); + +const channels: GraphQLChannels | undefined = dc && { + execute: dc.tracingChannel('graphql:execute'), + parse: dc.tracingChannel('graphql:parse'), + validate: dc.tracingChannel('graphql:validate'), + resolve: dc.tracingChannel('graphql:resolve'), + subscribe: dc.tracingChannel('graphql:subscribe'), +}; /** - * Internal accessor used at emission sites. Returns `undefined` when no - * `diagnostics_channel` module has been registered, allowing emission sites - * to short-circuit on a single property access. + * Internal accessor used at emission sites. Returns `undefined` when + * `node:diagnostics_channel` isn't available on this runtime, allowing + * emission sites to short-circuit on a single property access. * * @internal */ @@ -87,66 +116,13 @@ export function getChannels(): GraphQLChannels | undefined { return channels; } -/** - * Register a `node:diagnostics_channel`-compatible module with graphql-js. - * - * After calling this, graphql-js will publish lifecycle events on the - * following tracing channels whenever subscribers are present: - * - * - `graphql:parse` - * - `graphql:validate` - * - `graphql:execute` - * - `graphql:subscribe` - * - `graphql:resolve` - * - * Re-registration is tolerated when the incoming `dc` exposes the same - * `tracingChannel` function as the previously registered one - * @throws {Error} If a different `diagnostics_channel` module is registered. - * - * @example - * ```ts - * import dc from 'node:diagnostics_channel'; - * import { enableDiagnosticsChannel } from 'graphql'; - * - * try { - * enableDiagnosticsChannel(dc); - * } catch { - * // A diagnostic_channel module was already registered, safe to subscribe. - * } - * ``` - */ -export function enableDiagnosticsChannel(dc: MinimalDiagnosticsChannel): void { - if (registeredDc !== undefined) { - // Compare `tracingChannel` function identity rather than module identity - // so consumers that pass an ESM Module Namespace object and consumers - // that pass the default export of the same underlying module are - // treated as equivalent - if (registeredDc.tracingChannel !== dc.tracingChannel) { - throw new Error( - 'enableDiagnosticsChannel was called with a different `diagnostics_channel` module than the one previously registered. graphql-js can only publish to one module at a time; ensure all APMs share the same `node:diagnostics_channel` import.', - ); - } - return; - } - - registeredDc = dc; - channels = { - execute: dc.tracingChannel('graphql:execute'), - parse: dc.tracingChannel('graphql:parse'), - validate: dc.tracingChannel('graphql:validate'), - resolve: dc.tracingChannel('graphql:resolve'), - subscribe: dc.tracingChannel('graphql:subscribe'), - }; -} - /** * Gate for emission sites. Returns `true` when the named channel exists and * publishing should proceed. * * Uses `!== false` rather than a truthy check so runtimes which do not * implement the aggregated `hasSubscribers` getter on `TracingChannel` still - * publish. Notably Node 18 (nodejs/node#54470), where the aggregated getter - * returns `undefined` while sub-channels behave correctly. + * publish. * * @internal */ diff --git a/src/execution/__tests__/execute-diagnostics-test.ts b/src/execution/__tests__/execute-diagnostics-test.ts index e8ecea8266..fe4666c489 100644 --- a/src/execution/__tests__/execute-diagnostics-test.ts +++ b/src/execution/__tests__/execute-diagnostics-test.ts @@ -1,17 +1,15 @@ import { expect } from 'chai'; -import { afterEach, beforeEach, describe, it } from 'mocha'; +import { afterEach, describe, it } from 'mocha'; import { collectEvents, - sharedFakeDc, -} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + getTracingChannel, +} from '../../__testUtils__/diagnosticsTestUtils.js'; import { parse } from '../../language/parser.js'; import { buildSchema } from '../../utilities/buildASTSchema.js'; -import { enableDiagnosticsChannel } from '../../diagnostics.js'; - import type { ExecutionArgs } from '../execute.js'; import { execute, @@ -32,15 +30,11 @@ const rootValue = { async: () => Promise.resolve('hello-async'), }; -const executeChannel = sharedFakeDc.tracingChannel('graphql:execute'); +const executeChannel = getTracingChannel('graphql:execute'); describe('execute diagnostics channel', () => { let active: ReturnType | undefined; - beforeEach(() => { - enableDiagnosticsChannel(sharedFakeDc); - }); - afterEach(() => { active?.unsubscribe(); active = undefined; diff --git a/src/execution/__tests__/resolve-diagnostics-test.ts b/src/execution/__tests__/resolve-diagnostics-test.ts index c524a2c8f5..fc6a53cc17 100644 --- a/src/execution/__tests__/resolve-diagnostics-test.ts +++ b/src/execution/__tests__/resolve-diagnostics-test.ts @@ -1,10 +1,10 @@ import { expect } from 'chai'; -import { afterEach, beforeEach, describe, it } from 'mocha'; +import { afterEach, describe, it } from 'mocha'; import { collectEvents, - sharedFakeDc, -} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + getTracingChannel, +} from '../../__testUtils__/diagnosticsTestUtils.js'; import { isPromise } from '../../jsutils/isPromise.js'; @@ -16,8 +16,6 @@ import { GraphQLSchema } from '../../type/schema.js'; import { buildSchema } from '../../utilities/buildASTSchema.js'; -import { enableDiagnosticsChannel } from '../../diagnostics.js'; - import { execute } from '../execute.js'; const schema = buildSchema(` @@ -46,15 +44,11 @@ const rootValue = { nested: { leaf: 'leaf-value' }, }; -const resolveChannel = sharedFakeDc.tracingChannel('graphql:resolve'); +const resolveChannel = getTracingChannel('graphql:resolve'); describe('resolve diagnostics channel', () => { let active: ReturnType | undefined; - beforeEach(() => { - enableDiagnosticsChannel(sharedFakeDc); - }); - afterEach(() => { active?.unsubscribe(); active = undefined; diff --git a/src/execution/__tests__/subscribe-diagnostics-test.ts b/src/execution/__tests__/subscribe-diagnostics-test.ts index 99c497a613..7ea78d6836 100644 --- a/src/execution/__tests__/subscribe-diagnostics-test.ts +++ b/src/execution/__tests__/subscribe-diagnostics-test.ts @@ -1,10 +1,10 @@ import { expect } from 'chai'; -import { afterEach, beforeEach, describe, it } from 'mocha'; +import { afterEach, describe, it } from 'mocha'; import { collectEvents, - sharedFakeDc, -} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + getTracingChannel, +} from '../../__testUtils__/diagnosticsTestUtils.js'; import { isPromise } from '../../jsutils/isPromise.js'; @@ -14,8 +14,6 @@ import { GraphQLObjectType } from '../../type/definition.js'; import { GraphQLString } from '../../type/scalars.js'; import { GraphQLSchema } from '../../type/schema.js'; -import { enableDiagnosticsChannel } from '../../diagnostics.js'; - import { subscribe } from '../execute.js'; function buildSubscriptionSchema( @@ -44,15 +42,11 @@ async function* twoTicks(): AsyncIterable<{ tick: string }> { yield { tick: 'two' }; } -const subscribeChannel = sharedFakeDc.tracingChannel('graphql:subscribe'); +const subscribeChannel = getTracingChannel('graphql:subscribe'); describe('subscribe diagnostics channel', () => { let active: ReturnType | undefined; - beforeEach(() => { - enableDiagnosticsChannel(sharedFakeDc); - }); - afterEach(() => { active?.unsubscribe(); active = undefined; diff --git a/src/index.ts b/src/index.ts index ec1001325e..15564f2b50 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,16 +32,10 @@ export { version, versionInfo } from './version.js'; // Enable development mode for additional checks. export { enableDevMode, isDevModeEnabled } from './devMode.js'; -// Register a `node:diagnostics_channel`-compatible module to enable -// tracing channel emission from parse, validate, execute, subscribe, -// and resolver lifecycle events. -export { enableDiagnosticsChannel } from './diagnostics.js'; -export type { - MinimalChannel, - MinimalTracingChannel, - MinimalDiagnosticsChannel, - GraphQLChannels, -} from './diagnostics.js'; +// Tracing channel types for subscribers that want to strongly type the +// `graphql:*` channel context payloads. Channels are auto-registered on +// `node:diagnostics_channel` at module load. +export type { GraphQLChannels } from './diagnostics.js'; // The primary entry point into fulfilling a GraphQL request. export type { GraphQLArgs } from './graphql.js'; diff --git a/src/language/__tests__/parser-diagnostics-test.ts b/src/language/__tests__/parser-diagnostics-test.ts index 9ddfdf30d9..49535dacc7 100644 --- a/src/language/__tests__/parser-diagnostics-test.ts +++ b/src/language/__tests__/parser-diagnostics-test.ts @@ -1,24 +1,18 @@ import { expect } from 'chai'; -import { afterEach, beforeEach, describe, it } from 'mocha'; +import { afterEach, describe, it } from 'mocha'; import { collectEvents, - sharedFakeDc, -} from '../../__testUtils__/fakeDiagnosticsChannel.js'; - -import { enableDiagnosticsChannel } from '../../diagnostics.js'; + getTracingChannel, +} from '../../__testUtils__/diagnosticsTestUtils.js'; import { parse } from '../parser.js'; -const parseChannel = sharedFakeDc.tracingChannel('graphql:parse'); +const parseChannel = getTracingChannel('graphql:parse'); describe('parse diagnostics channel', () => { let active: ReturnType | undefined; - beforeEach(() => { - enableDiagnosticsChannel(sharedFakeDc); - }); - afterEach(() => { active?.unsubscribe(); active = undefined; diff --git a/src/validation/__tests__/validate-diagnostics-test.ts b/src/validation/__tests__/validate-diagnostics-test.ts index 622d44c5f8..85c308ab6a 100644 --- a/src/validation/__tests__/validate-diagnostics-test.ts +++ b/src/validation/__tests__/validate-diagnostics-test.ts @@ -1,17 +1,15 @@ import { expect } from 'chai'; -import { afterEach, beforeEach, describe, it } from 'mocha'; +import { afterEach, describe, it } from 'mocha'; import { collectEvents, - sharedFakeDc, -} from '../../__testUtils__/fakeDiagnosticsChannel.js'; + getTracingChannel, +} from '../../__testUtils__/diagnosticsTestUtils.js'; import { parse } from '../../language/parser.js'; import { buildSchema } from '../../utilities/buildASTSchema.js'; -import { enableDiagnosticsChannel } from '../../diagnostics.js'; - import { validate } from '../validate.js'; const schema = buildSchema(` @@ -20,15 +18,11 @@ const schema = buildSchema(` } `); -const validateChannel = sharedFakeDc.tracingChannel('graphql:validate'); +const validateChannel = getTracingChannel('graphql:validate'); describe('validate diagnostics channel', () => { let active: ReturnType | undefined; - beforeEach(() => { - enableDiagnosticsChannel(sharedFakeDc); - }); - afterEach(() => { active?.unsubscribe(); active = undefined; From 68d0f576e70b67bd3b628c1de026501118b75d9f Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 24 Apr 2026 09:39:49 -0400 Subject: [PATCH 18/29] ref: create direct pointers to tracing channels --- src/__tests__/diagnostics-test.ts | 42 ++++++++----- src/diagnostics.ts | 90 +++++++++++----------------- src/language/parser.ts | 32 +++++----- src/validation/validate.ts | 97 +++++++++++++++++-------------- 4 files changed, 132 insertions(+), 129 deletions(-) diff --git a/src/__tests__/diagnostics-test.ts b/src/__tests__/diagnostics-test.ts index 9c62ce9b9c..c7c97b0c60 100644 --- a/src/__tests__/diagnostics-test.ts +++ b/src/__tests__/diagnostics-test.ts @@ -6,26 +6,38 @@ import { describe, it } from 'mocha'; import { invariant } from '../jsutils/invariant.js'; -import { getChannels } from '../diagnostics.js'; +import { + executeChannel, + parseChannel, + resolveChannel, + subscribeChannel, + validateChannel, +} from '../diagnostics.js'; describe('diagnostics', () => { it('auto-registers the five graphql tracing channels', () => { - const channels = getChannels(); - invariant(channels !== undefined); + invariant(parseChannel !== undefined); + invariant(validateChannel !== undefined); + invariant(executeChannel !== undefined); + invariant(subscribeChannel !== undefined); + invariant(resolveChannel !== undefined); // Node's `tracingChannel(name)` returns a fresh wrapper per call but // the underlying sub-channels are cached by name, so compare those. - const byName = { - execute: 'graphql:execute', - parse: 'graphql:parse', - validate: 'graphql:validate', - resolve: 'graphql:resolve', - subscribe: 'graphql:subscribe', - } as const; - for (const [key, name] of Object.entries(byName)) { - expect(channels[key as keyof typeof byName].start).to.equal( - dc.channel(`tracing:${name}:start`), - ); - } + expect(parseChannel.start).to.equal( + dc.channel('tracing:graphql:parse:start'), + ); + expect(validateChannel.start).to.equal( + dc.channel('tracing:graphql:validate:start'), + ); + expect(executeChannel.start).to.equal( + dc.channel('tracing:graphql:execute:start'), + ); + expect(subscribeChannel.start).to.equal( + dc.channel('tracing:graphql:subscribe:start'), + ); + expect(resolveChannel.start).to.equal( + dc.channel('tracing:graphql:resolve:start'), + ); }); }); diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 9e731885c8..d194d0debc 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -80,7 +80,9 @@ function resolveDiagnosticsChannel(): DiagnosticsChannelModule | undefined { .getBuiltinModule === 'function' ) { dc = ( - process as { getBuiltinModule: (id: string) => DiagnosticsChannelModule } + process as { + getBuiltinModule: (id: string) => DiagnosticsChannelModule; + } ).getBuiltinModule('node:diagnostics_channel'); } if (!dc && typeof require === 'function') { @@ -97,80 +99,56 @@ function resolveDiagnosticsChannel(): DiagnosticsChannelModule | undefined { const dc = resolveDiagnosticsChannel(); -const channels: GraphQLChannels | undefined = dc && { - execute: dc.tracingChannel('graphql:execute'), - parse: dc.tracingChannel('graphql:parse'), - validate: dc.tracingChannel('graphql:validate'), - resolve: dc.tracingChannel('graphql:resolve'), - subscribe: dc.tracingChannel('graphql:subscribe'), -}; - /** - * Internal accessor used at emission sites. Returns `undefined` when - * `node:diagnostics_channel` isn't available on this runtime, allowing - * emission sites to short-circuit on a single property access. + * Per-channel handles, resolved once at module load. `undefined` when + * `node:diagnostics_channel` isn't available. Emission sites read these + * directly to keep the no-subscriber fast path to a single property access + * plus a `hasSubscribers` check (no function calls, no closures). * * @internal */ -export function getChannels(): GraphQLChannels | undefined { - return channels; -} +export const parseChannel: MinimalTracingChannel | undefined = + dc?.tracingChannel('graphql:parse'); +/** @internal */ +export const validateChannel: MinimalTracingChannel | undefined = + dc?.tracingChannel('graphql:validate'); +/** @internal */ +export const executeChannel: MinimalTracingChannel | undefined = + dc?.tracingChannel('graphql:execute'); +/** @internal */ +export const subscribeChannel: MinimalTracingChannel | undefined = + dc?.tracingChannel('graphql:subscribe'); +/** @internal */ +export const resolveChannel: MinimalTracingChannel | undefined = + dc?.tracingChannel('graphql:resolve'); /** - * Gate for emission sites. Returns `true` when the named channel exists and - * publishing should proceed. - * - * Uses `!== false` rather than a truthy check so runtimes which do not - * implement the aggregated `hasSubscribers` getter on `TracingChannel` still - * publish. + * Publish a synchronous operation through `channel`. Caller has already + * verified that a subscriber is attached; this helper exists only so the + * traced path doesn't need to be duplicated at every emission site. * * @internal */ -export function shouldTrace( - channel: MinimalTracingChannel | undefined, -): channel is MinimalTracingChannel { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare - return channel !== undefined && channel.hasSubscribers !== false; -} - -/** - * Publish a synchronous operation through the named graphql tracing channel, - * short-circuiting to `fn()` when the channel isn't registered or nothing is - * listening. - * - * @internal - */ -export function maybeTraceSync( - name: keyof GraphQLChannels, - ctxFactory: () => object, +export function traceSync( + channel: MinimalTracingChannel, + ctx: object, fn: () => T, ): T { - const channel = getChannels()?.[name]; - if (!shouldTrace(channel)) { - return fn(); - } - return channel.traceSync(fn, ctxFactory()); + return channel.traceSync(fn, ctx); } /** - * Publish a mixed sync-or-promise operation through the named graphql tracing - * channel. + * Publish a mixed sync-or-promise operation through `channel`. Caller has + * already verified that a subscriber is attached. * * @internal */ -export function maybeTraceMixed( - name: keyof GraphQLChannels, - ctxFactory: () => object, +export function traceMixed( + channel: MinimalTracingChannel, + ctxInput: object, fn: () => T | Promise, ): T | Promise { - const channel = getChannels()?.[name]; - if (!shouldTrace(channel)) { - return fn(); - } - const ctx = ctxFactory() as { - error?: unknown; - result?: unknown; - }; + const ctx = ctxInput as { error?: unknown; result?: unknown }; return channel.start.runStores(ctx, () => { let result: T | Promise; diff --git a/src/language/parser.ts b/src/language/parser.ts index ba62a9ccb0..0afb5685a3 100644 --- a/src/language/parser.ts +++ b/src/language/parser.ts @@ -3,7 +3,7 @@ import type { Maybe } from '../jsutils/Maybe.js'; import type { GraphQLError } from '../error/GraphQLError.js'; import { syntaxError } from '../error/syntaxError.js'; -import { maybeTraceSync } from '../diagnostics.js'; +import { parseChannel } from '../diagnostics.js'; import type { ArgumentCoordinateNode, @@ -134,19 +134,23 @@ export function parse( source: string | Source, options?: ParseOptions, ): DocumentNode { - return maybeTraceSync( - 'parse', - () => ({ source }), - () => { - const parser = new Parser(source, options); - const document = parser.parseDocument(); - Object.defineProperty(document, 'tokenCount', { - enumerable: false, - value: parser.tokenCount, - }); - return document; - }, - ); + if (!parseChannel?.hasSubscribers) { + return parseImpl(source, options); + } + return parseChannel.traceSync(() => parseImpl(source, options), { source }); +} + +function parseImpl( + source: string | Source, + options: ParseOptions | undefined, +): DocumentNode { + const parser = new Parser(source, options); + const document = parser.parseDocument(); + Object.defineProperty(document, 'tokenCount', { + enumerable: false, + value: parser.tokenCount, + }); + return document; } /** diff --git a/src/validation/validate.ts b/src/validation/validate.ts index 0c6a7499e6..7cd74fc8b6 100644 --- a/src/validation/validate.ts +++ b/src/validation/validate.ts @@ -12,7 +12,7 @@ import { assertValidSchema } from '../type/validate.js'; import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js'; -import { maybeTraceSync } from '../diagnostics.js'; +import { validateChannel } from '../diagnostics.js'; import { specifiedRules, specifiedSDLRules } from './specifiedRules.js'; import type { SDLValidationRule, ValidationRule } from './ValidationContext.js'; @@ -63,52 +63,61 @@ export function validate( rules: ReadonlyArray = specifiedRules, options?: ValidationOptions, ): ReadonlyArray { - return maybeTraceSync( - 'validate', - () => ({ schema, document: documentAST }), - () => { - const maxErrors = options?.maxErrors ?? 100; - const hideSuggestions = options?.hideSuggestions ?? false; - - // If the schema used for validation is invalid, throw an error. - assertValidSchema(schema); - - const errors: Array = []; - const typeInfo = new TypeInfo(schema); - const context = new ValidationContext( - schema, - documentAST, - typeInfo, - (error) => { - if (errors.length >= maxErrors) { - throw tooManyValidationErrorsError; - } - errors.push(error); - }, - hideSuggestions, - ); - - // This uses a specialized visitor which runs multiple visitors in - // parallel, while maintaining the visitor skip and break API. - const visitor = visitInParallel(rules.map((rule) => rule(context))); - - // Visit the whole document with each instance of all provided rules. - try { - visit( - documentAST, - visitWithTypeInfo(typeInfo, visitor), - QueryDocumentKeysToValidate, - ); - } catch (e: unknown) { - if (e === tooManyValidationErrorsError) { - errors.push(tooManyValidationErrorsError); - } else { - throw e; - } + if (!validateChannel?.hasSubscribers) { + return validateImpl(schema, documentAST, rules, options); + } + return validateChannel.traceSync( + () => validateImpl(schema, documentAST, rules, options), + { schema, document: documentAST }, + ); +} + +function validateImpl( + schema: GraphQLSchema, + documentAST: DocumentNode, + rules: ReadonlyArray, + options: ValidationOptions | undefined, +): ReadonlyArray { + const maxErrors = options?.maxErrors ?? 100; + const hideSuggestions = options?.hideSuggestions ?? false; + + // If the schema used for validation is invalid, throw an error. + assertValidSchema(schema); + + const errors: Array = []; + const typeInfo = new TypeInfo(schema); + const context = new ValidationContext( + schema, + documentAST, + typeInfo, + (error) => { + if (errors.length >= maxErrors) { + throw tooManyValidationErrorsError; } - return errors; + errors.push(error); }, + hideSuggestions, ); + + // This uses a specialized visitor which runs multiple visitors in + // parallel, while maintaining the visitor skip and break API. + const visitor = visitInParallel(rules.map((rule) => rule(context))); + + // Visit the whole document with each instance of all provided rules. + try { + visit( + documentAST, + visitWithTypeInfo(typeInfo, visitor), + QueryDocumentKeysToValidate, + ); + } catch (e: unknown) { + if (e === tooManyValidationErrorsError) { + errors.push(tooManyValidationErrorsError); + } else { + throw e; + } + } + return errors; } /** From 03c9fea38c89b382d71c81489514f8393d19d122 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 24 Apr 2026 10:13:36 -0400 Subject: [PATCH 19/29] ref(perf): inline no-subscriber fast path at tracing emission sites On parse/validate/execute/subscribe entry points, the previous `maybeTrace*` wrapper allocated a ctx-factory closure and a body closure on every call before short-circuiting on `hasSubscribers`. Inline the gate as `if (!channel?.hasSubscribers) return impl(args)` so closures and ctx objects are only allocated when a subscriber is attached. Recovers roughly half of the tracing-channel overhead seen in benchmarks. List-sync is back within noise of the pre-tracing baseline. Introspection remains ~3% slower due to the per-field resolve gate; the shape-change fix (dropping `_resolveChannel` from Executor) in the previous commit already captured the other half of that regression. --- src/diagnostics.ts | 10 ++ src/execution/Executor.ts | 22 +--- src/execution/execute.ts | 204 ++++++++++++++++++++++--------------- src/language/parser.ts | 9 +- src/validation/validate.ts | 15 ++- 5 files changed, 147 insertions(+), 113 deletions(-) diff --git a/src/diagnostics.ts b/src/diagnostics.ts index d194d0debc..c0fb84e34e 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -187,3 +187,13 @@ export function traceMixed( }); }); } + +/** + * Check if a channel is defined and has subscribers. + */ +export function shouldTrace( + channel: MinimalTracingChannel | undefined, +): channel is MinimalTracingChannel { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare + return channel !== undefined && channel.hasSubscribers !== false; +} diff --git a/src/execution/Executor.ts b/src/execution/Executor.ts index f277313e42..08c4d27c1a 100644 --- a/src/execution/Executor.ts +++ b/src/execution/Executor.ts @@ -44,8 +44,7 @@ import { } from '../type/definition.js'; import type { GraphQLSchema } from '../type/schema.js'; -import type { MinimalTracingChannel } from '../diagnostics.js'; -import { getChannels, maybeTraceMixed, shouldTrace } from '../diagnostics.js'; +import { resolveChannel, shouldTrace, traceMixed } from '../diagnostics.js'; import { AbortedGraphQLExecutionError } from './AbortedGraphQLExecutionError.js'; import { withCancellation } from './cancellablePromise.js'; @@ -246,12 +245,6 @@ export class Executor< values: ReadonlyArray>, ) => Promise>; - // Resolved once per Executor so the per-field gate in `executeField` is a - // single member read + null check, not a `getChannels()?.resolve` walk + - // `hasSubscribers` read on every resolution. Undefined when diagnostics - // are off or nobody is listening at construction time. - _resolveChannel: MinimalTracingChannel | undefined; - constructor( validatedExecutionArgs: ValidatedExecutionArgs, sharedExecutionContext?: SharedExecutionContext, @@ -261,11 +254,6 @@ export class Executor< this.abortReason = defaultAbortReason; this.collectedErrors = new CollectedErrors(); - const resolveChannel = getChannels()?.resolve; - this._resolveChannel = shouldTrace(resolveChannel) - ? resolveChannel - : undefined; - if (sharedExecutionContext === undefined) { this.resolverAbortController = new AbortController(); this.sharedExecutionContext = createSharedExecutionContext( @@ -627,10 +615,10 @@ export class Executor< // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly // used to represent an authenticated user, or request-specific caches. - const result = this._resolveChannel - ? maybeTraceMixed( - 'resolve', - () => buildResolveCtx(info, args, fieldDef.resolve === undefined), + const result = shouldTrace(resolveChannel) + ? traceMixed( + resolveChannel, + buildResolveCtx(info, args, fieldDef.resolve === undefined), () => resolveFn(source, args, contextValue, info), ) : resolveFn(source, args, contextValue, info); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index c818401850..9ce5bb54b0 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -34,7 +34,11 @@ import type { GraphQLSchema } from '../type/schema.js'; import { getOperationAST } from '../utilities/getOperationAST.js'; -import { maybeTraceMixed } from '../diagnostics.js'; +import { + executeChannel, + subscribeChannel, + traceMixed, +} from '../diagnostics.js'; import { cancellablePromise } from './cancellablePromise.js'; import type { FieldDetailsList, FragmentDetails } from './collectFields.js'; @@ -58,26 +62,24 @@ const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = * resolution of the operation AST to a lazy getter so the cost of walking * the document is only paid if a subscriber reads it. */ -function buildExecuteCtxFromArgs(args: ExecutionArgs): () => object { - return () => { - let operation: OperationDefinitionNode | null | undefined; - const resolveOperation = (): OperationDefinitionNode | null | undefined => { - if (operation === undefined) { - operation = getOperationAST(args.document, args.operationName); - } - return operation; - }; - return { - document: args.document, - schema: args.schema, - variableValues: args.variableValues, - get operationName() { - return args.operationName ?? resolveOperation()?.name?.value; - }, - get operationType() { - return resolveOperation()?.operation; - }, - }; +function buildExecuteCtxFromArgs(args: ExecutionArgs): object { + let operation: OperationDefinitionNode | null | undefined; + const resolveOperation = (): OperationDefinitionNode | null | undefined => { + if (operation === undefined) { + operation = getOperationAST(args.document, args.operationName); + } + return operation; + }; + return { + document: args.document, + schema: args.schema, + variableValues: args.variableValues, + get operationName() { + return args.operationName ?? resolveOperation()?.name?.value; + }, + get operationType() { + return resolveOperation()?.operation; + }, }; } @@ -90,14 +92,14 @@ function buildExecuteCtxFromArgs(args: ExecutionArgs): () => object { */ function buildExecuteCtxFromValidatedArgs( args: ValidatedExecutionArgs, -): () => object { - return () => ({ +): object { + return { operation: args.operation, schema: args.schema, variableValues: args.variableValues, operationName: args.operation.name?.value, operationType: args.operation.operation, - }); + }; } /** @@ -117,23 +119,27 @@ function buildExecuteCtxFromValidatedArgs( * delivery. */ export function execute(args: ExecutionArgs): PromiseOrValue { - return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => { - if ( - args.schema.getDirective('defer') || - args.schema.getDirective('stream') - ) { - throw new Error(UNEXPECTED_EXPERIMENTAL_DIRECTIVES); - } + if (!executeChannel?.hasSubscribers) { + return executeImpl(args); + } + return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () => + executeImpl(args), + ); +} - const validatedExecutionArgs = validateExecutionArgs(args); +function executeImpl(args: ExecutionArgs): PromiseOrValue { + if (args.schema.getDirective('defer') || args.schema.getDirective('stream')) { + throw new Error(UNEXPECTED_EXPERIMENTAL_DIRECTIVES); + } - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + const validatedExecutionArgs = validateExecutionArgs(args); - return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); - }); + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } + + return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); } /** @@ -151,39 +157,57 @@ export function execute(args: ExecutionArgs): PromiseOrValue { export function experimentalExecuteIncrementally( args: ExecutionArgs, ): PromiseOrValue { - return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => { - // If a valid execution context cannot be created due to incorrect - // arguments, a "Response" with only errors is returned. - const validatedExecutionArgs = validateExecutionArgs(args); - - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + if (!executeChannel?.hasSubscribers) { + return experimentalExecuteIncrementallyImpl(args); + } + return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () => + experimentalExecuteIncrementallyImpl(args), + ); +} - return experimentalExecuteQueryOrMutationOrSubscriptionEvent( - validatedExecutionArgs, - ); - }); +function experimentalExecuteIncrementallyImpl( + args: ExecutionArgs, +): PromiseOrValue { + // If a valid execution context cannot be created due to incorrect + // arguments, a "Response" with only errors is returned. + const validatedExecutionArgs = validateExecutionArgs(args); + + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } + + return experimentalExecuteQueryOrMutationOrSubscriptionEvent( + validatedExecutionArgs, + ); } export function executeIgnoringIncremental( args: ExecutionArgs, ): PromiseOrValue { - return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => { - // If a valid execution context cannot be created due to incorrect - // arguments, a "Response" with only errors is returned. - const validatedExecutionArgs = validateExecutionArgs(args); - - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + if (!executeChannel?.hasSubscribers) { + return executeIgnoringIncrementalImpl(args); + } + return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () => + executeIgnoringIncrementalImpl(args), + ); +} - return executeQueryOrMutationOrSubscriptionEventIgnoringIncremental( - validatedExecutionArgs, - ); - }); +function executeIgnoringIncrementalImpl( + args: ExecutionArgs, +): PromiseOrValue { + // If a valid execution context cannot be created due to incorrect + // arguments, a "Response" with only errors is returned. + const validatedExecutionArgs = validateExecutionArgs(args); + + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } + + return executeQueryOrMutationOrSubscriptionEventIgnoringIncremental( + validatedExecutionArgs, + ); } /** @@ -244,8 +268,11 @@ export function executeSync(args: ExecutionArgs): ExecutionResult { export function executeSubscriptionEvent( validatedExecutionArgs: ValidatedExecutionArgs, ): PromiseOrValue { - return maybeTraceMixed( - 'execute', + if (!executeChannel?.hasSubscribers) { + return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); + } + return traceMixed( + executeChannel, buildExecuteCtxFromValidatedArgs(validatedExecutionArgs), () => executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs), ); @@ -282,26 +309,37 @@ export function subscribe( ): PromiseOrValue< AsyncGenerator | ExecutionResult > { - return maybeTraceMixed('subscribe', buildExecuteCtxFromArgs(args), () => { - // If a valid execution context cannot be created due to incorrect - // arguments, a "Response" with only errors is returned. - const validatedExecutionArgs = validateExecutionArgs(args); - - // Return early errors if execution context failed. - if (!('schema' in validatedExecutionArgs)) { - return { errors: validatedExecutionArgs }; - } + if (!subscribeChannel?.hasSubscribers) { + return subscribeImpl(args); + } + return traceMixed(subscribeChannel, buildExecuteCtxFromArgs(args), () => + subscribeImpl(args), + ); +} - const resultOrStream = createSourceEventStreamImpl(validatedExecutionArgs); +function subscribeImpl( + args: ExecutionArgs, +): PromiseOrValue< + AsyncGenerator | ExecutionResult +> { + // If a valid execution context cannot be created due to incorrect + // arguments, a "Response" with only errors is returned. + const validatedExecutionArgs = validateExecutionArgs(args); - if (isPromise(resultOrStream)) { - return resultOrStream.then((resolvedResultOrStream) => - mapSourceToResponse(validatedExecutionArgs, resolvedResultOrStream), - ); - } + // Return early errors if execution context failed. + if (!('schema' in validatedExecutionArgs)) { + return { errors: validatedExecutionArgs }; + } + + const resultOrStream = createSourceEventStreamImpl(validatedExecutionArgs); + + if (isPromise(resultOrStream)) { + return resultOrStream.then((resolvedResultOrStream) => + mapSourceToResponse(validatedExecutionArgs, resolvedResultOrStream), + ); + } - return mapSourceToResponse(validatedExecutionArgs, resultOrStream); - }); + return mapSourceToResponse(validatedExecutionArgs, resultOrStream); } /** diff --git a/src/language/parser.ts b/src/language/parser.ts index 0afb5685a3..9439a32b9f 100644 --- a/src/language/parser.ts +++ b/src/language/parser.ts @@ -3,7 +3,7 @@ import type { Maybe } from '../jsutils/Maybe.js'; import type { GraphQLError } from '../error/GraphQLError.js'; import { syntaxError } from '../error/syntaxError.js'; -import { parseChannel } from '../diagnostics.js'; +import { parseChannel, shouldTrace } from '../diagnostics.js'; import type { ArgumentCoordinateNode, @@ -134,10 +134,9 @@ export function parse( source: string | Source, options?: ParseOptions, ): DocumentNode { - if (!parseChannel?.hasSubscribers) { - return parseImpl(source, options); - } - return parseChannel.traceSync(() => parseImpl(source, options), { source }); + return shouldTrace(parseChannel) + ? parseChannel.traceSync(() => parseImpl(source, options), { source }) + : parseImpl(source, options); } function parseImpl( diff --git a/src/validation/validate.ts b/src/validation/validate.ts index 7cd74fc8b6..48b79a4b72 100644 --- a/src/validation/validate.ts +++ b/src/validation/validate.ts @@ -12,7 +12,7 @@ import { assertValidSchema } from '../type/validate.js'; import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js'; -import { validateChannel } from '../diagnostics.js'; +import { shouldTrace, validateChannel } from '../diagnostics.js'; import { specifiedRules, specifiedSDLRules } from './specifiedRules.js'; import type { SDLValidationRule, ValidationRule } from './ValidationContext.js'; @@ -63,13 +63,12 @@ export function validate( rules: ReadonlyArray = specifiedRules, options?: ValidationOptions, ): ReadonlyArray { - if (!validateChannel?.hasSubscribers) { - return validateImpl(schema, documentAST, rules, options); - } - return validateChannel.traceSync( - () => validateImpl(schema, documentAST, rules, options), - { schema, document: documentAST }, - ); + return shouldTrace(validateChannel) + ? validateChannel.traceSync( + () => validateImpl(schema, documentAST, rules, options), + { schema, document: documentAST }, + ) + : validateImpl(schema, documentAST, rules, options); } function validateImpl( From bbddc0cc86357d0fcbf7a4f83ee78ab8fa2c68dd Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 24 Apr 2026 11:33:33 -0400 Subject: [PATCH 20/29] test: cover executeIgnoringIncremental traced path and drop unused traceSync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The traced branch of `executeIgnoringIncremental` was not exercised by any subscriber-attached test, tripping the 100% coverage threshold on CI. Add a case in execute-diagnostics-test that subscribes to `graphql:execute` and asserts start/end around a call. Also drop the `traceSync` helper from diagnostics.ts; every call site invokes `channel.traceSync(...)` directly, so the export was dead code. Mark the `require('node:diagnostics_channel')` CJS fallback and the enclosing `catch` block with c8 ignore comments — they are only reachable on Node 20.0-20.15 (pre-`getBuiltinModule`) and on runtimes without `diagnostics_channel` at all, neither of which the unit tests run on. --- src/diagnostics.ts | 17 ++--------------- .../__tests__/execute-diagnostics-test.ts | 12 ++++++++++++ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/diagnostics.ts b/src/diagnostics.ts index c0fb84e34e..de11c758cf 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -85,12 +85,14 @@ function resolveDiagnosticsChannel(): DiagnosticsChannelModule | undefined { } ).getBuiltinModule('node:diagnostics_channel'); } + /* c8 ignore next 6 */ if (!dc && typeof require === 'function') { // CJS fallback for runtimes that lack `process.getBuiltinModule` // (e.g. Node 20.0 - 20.15). ESM builds skip this branch because // `require` is undeclared there. dc = require('node:diagnostics_channel') as DiagnosticsChannelModule; } + /* c8 ignore next 3 */ } catch { // diagnostics_channel not available on this runtime; tracing is a no-op. } @@ -122,21 +124,6 @@ export const subscribeChannel: MinimalTracingChannel | undefined = export const resolveChannel: MinimalTracingChannel | undefined = dc?.tracingChannel('graphql:resolve'); -/** - * Publish a synchronous operation through `channel`. Caller has already - * verified that a subscriber is attached; this helper exists only so the - * traced path doesn't need to be duplicated at every emission site. - * - * @internal - */ -export function traceSync( - channel: MinimalTracingChannel, - ctx: object, - fn: () => T, -): T { - return channel.traceSync(fn, ctx); -} - /** * Publish a mixed sync-or-promise operation through `channel`. Caller has * already verified that a subscriber is attached. diff --git a/src/execution/__tests__/execute-diagnostics-test.ts b/src/execution/__tests__/execute-diagnostics-test.ts index fe4666c489..896189c7a3 100644 --- a/src/execution/__tests__/execute-diagnostics-test.ts +++ b/src/execution/__tests__/execute-diagnostics-test.ts @@ -13,6 +13,7 @@ import { buildSchema } from '../../utilities/buildASTSchema.js'; import type { ExecutionArgs } from '../execute.js'; import { execute, + executeIgnoringIncremental, executeSubscriptionEvent, executeSync, validateExecutionArgs, @@ -78,6 +79,17 @@ describe('execute diagnostics channel', () => { expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); }); + it('emits start and end around executeIgnoringIncremental', () => { + active = collectEvents(executeChannel); + + const document = parse('query Q { sync }'); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + executeIgnoringIncremental({ schema, document, rootValue }); + + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + expect(active.events[0].ctx.operationName).to.equal('Q'); + }); + it('emits start, error, and end when execute throws synchronously', () => { active = collectEvents(executeChannel); From 13ccd027e1eff21da2c292c5571368b5410347bb Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 24 Apr 2026 18:48:45 -0400 Subject: [PATCH 21/29] ref(perf): keep executeField inlinable by extracting traced path Hoist the resolve tracing decision once per `executeFields` / `executeFieldsSerially` call instead of re-checking `shouldTrace(resolveChannel)` per field, and move the traced branch's `traceMixed` call + ctx closure into a module-scope helper `invokeResolverWithTracing`. The latter is the load-bearing change: once `executeField`'s body referenced `traceMixed` and the `() => resolveFn(...)` closure, V8 stopped inlining `executeField` into `executeFields`, paying a real call frame per field. Extracting the two-function tail keeps `executeField`'s bytecode small enough to inline again. Effect on the introspection benchmark (the worst-case fan-out shape, ~1000 resolver calls per iteration, no subscribers attached): - with the inline ternary: -5.94% vs upstream - with this refactor: -2.05% / -2.30% across two paired runs Other benchmarks (list-sync, list-async, list-asyncIterable, object-async) sit at the noise floor with 95% CIs that cross zero. Adds a serial-mutation case to resolve-diagnostics-test to cover the `executeFieldsSerially` branch the hoisted snapshot now lives in. --- src/execution/Executor.ts | 45 ++++++++++++++++--- .../__tests__/resolve-diagnostics-test.ts | 29 ++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/execution/Executor.ts b/src/execution/Executor.ts index 08c4d27c1a..5b7ca46721 100644 --- a/src/execution/Executor.ts +++ b/src/execution/Executor.ts @@ -44,6 +44,7 @@ import { } from '../type/definition.js'; import type { GraphQLSchema } from '../type/schema.js'; +import type { MinimalTracingChannel } from '../diagnostics.js'; import { resolveChannel, shouldTrace, traceMixed } from '../diagnostics.js'; import { AbortedGraphQLExecutionError } from './AbortedGraphQLExecutionError.js'; @@ -480,6 +481,9 @@ export class Executor< groupedFieldSet: GroupedFieldSet, positionContext: TPositionContext | undefined, ): PromiseOrValue> { + const tracingChannel = shouldTrace(resolveChannel) + ? resolveChannel + : undefined; return promiseReduce( groupedFieldSet, (results, [responseName, fieldDetailsList]) => { @@ -493,6 +497,7 @@ export class Executor< fieldDetailsList, fieldPath, positionContext, + tracingChannel, ); if (result === undefined) { return results; @@ -523,6 +528,9 @@ export class Executor< ): PromiseOrValue> { const results = Object.create(null); let containsPromise = false; + const tracingChannel = shouldTrace(resolveChannel) + ? resolveChannel + : undefined; try { for (const [responseName, fieldDetailsList] of groupedFieldSet) { @@ -533,6 +541,7 @@ export class Executor< fieldDetailsList, fieldPath, positionContext, + tracingChannel, ); if (result !== undefined) { @@ -574,6 +583,7 @@ export class Executor< fieldDetailsList: FieldDetailsList, path: Path, positionContext: TPositionContext | undefined, + tracingChannel: MinimalTracingChannel | undefined, ): PromiseOrValue { const validatedExecutionArgs = this.validatedExecutionArgs; const { schema, contextValue, variableValues, hideSuggestions } = @@ -615,11 +625,15 @@ export class Executor< // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly // used to represent an authenticated user, or request-specific caches. - const result = shouldTrace(resolveChannel) - ? traceMixed( - resolveChannel, - buildResolveCtx(info, args, fieldDef.resolve === undefined), - () => resolveFn(source, args, contextValue, info), + const result = tracingChannel + ? invokeResolverWithTracing( + tracingChannel, + resolveFn, + source, + args, + contextValue, + info, + fieldDef.resolve === undefined, ) : resolveFn(source, args, contextValue, info); @@ -1430,3 +1444,24 @@ function buildResolveCtx( }, }; } + +/** + * Traced path for a single resolver call. Extracted as a module-scope function to increase likelihood of inlining. + * + * @internal + */ +function invokeResolverWithTracing( + tracingChannel: MinimalTracingChannel, + resolveFn: GraphQLFieldResolver, + source: unknown, + args: { readonly [argument: string]: unknown }, + contextValue: unknown, + info: GraphQLResolveInfo, + isTrivialResolver: boolean, +): PromiseOrValue { + return traceMixed( + tracingChannel, + buildResolveCtx(info, args, isTrivialResolver), + () => resolveFn(source, args, contextValue, info), + ); +} diff --git a/src/execution/__tests__/resolve-diagnostics-test.ts b/src/execution/__tests__/resolve-diagnostics-test.ts index fc6a53cc17..75fe68fe35 100644 --- a/src/execution/__tests__/resolve-diagnostics-test.ts +++ b/src/execution/__tests__/resolve-diagnostics-test.ts @@ -178,6 +178,35 @@ describe('resolve diagnostics channel', () => { expect(endsSync.length).to.equal(4); }); + it('emits per-field for serial mutation execution', async () => { + const mutationSchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { dummy: { type: GraphQLString } }, + }), + mutation: new GraphQLObjectType({ + name: 'Mutation', + fields: { + first: { type: GraphQLString, resolve: () => 'one' }, + second: { type: GraphQLString, resolve: () => 'two' }, + }, + }), + }); + + active = collectEvents(resolveChannel); + + await execute({ + schema: mutationSchema, + document: parse('mutation M { first second }'), + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + expect(starts.map((e) => e.ctx.fieldName)).to.deep.equal([ + 'first', + 'second', + ]); + }); + it('does nothing when no subscribers are attached', () => { const result = execute({ schema, From eea1a8e90aabff4aa439cb40912cb86a4d644f52 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 22:35:17 +0300 Subject: [PATCH 22/29] rename from isTrivialResolver => isDefaultResolver --- integrationTests/diagnostics/test.js | 4 ++-- src/execution/Executor.ts | 8 ++++---- src/execution/__tests__/resolve-diagnostics-test.ts | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/integrationTests/diagnostics/test.js b/integrationTests/diagnostics/test.js index 8a0e5cebb4..dd1d8e71ba 100644 --- a/integrationTests/diagnostics/test.js +++ b/integrationTests/diagnostics/test.js @@ -205,7 +205,7 @@ function runResolveCase() { parentType: msg.parentType, fieldType: msg.fieldType, fieldPath: msg.fieldPath, - isTrivialResolver: msg.isTrivialResolver, + isDefaultResolver: msg.isDefaultResolver, }), end: () => events.push({ kind: 'end' }), asyncStart: () => events.push({ kind: 'asyncStart' }), @@ -228,7 +228,7 @@ function runResolveCase() { assert.equal(hello.parentType, 'Query'); assert.equal(hello.fieldType, 'String'); // buildSchema never attaches field.resolve; all fields report as trivial. - assert.equal(hello.isTrivialResolver, true); + assert.equal(hello.isDefaultResolver, true); } finally { channel.unsubscribe(handler); } diff --git a/src/execution/Executor.ts b/src/execution/Executor.ts index 5b7ca46721..276deddcf0 100644 --- a/src/execution/Executor.ts +++ b/src/execution/Executor.ts @@ -1429,7 +1429,7 @@ function toNodes(fieldDetailsList: FieldDetailsList): ReadonlyArray { function buildResolveCtx( info: GraphQLResolveInfo, args: { readonly [argument: string]: unknown }, - isTrivialResolver: boolean, + isDefaultResolver: boolean, ): object { let cachedFieldPath: string | undefined; return { @@ -1437,7 +1437,7 @@ function buildResolveCtx( parentType: info.parentType.name, fieldType: String(info.returnType), args, - isTrivialResolver, + isDefaultResolver, get fieldPath() { cachedFieldPath ??= pathToArray(info.path).join('.'); return cachedFieldPath; @@ -1457,11 +1457,11 @@ function invokeResolverWithTracing( args: { readonly [argument: string]: unknown }, contextValue: unknown, info: GraphQLResolveInfo, - isTrivialResolver: boolean, + isDefaultResolver: boolean, ): PromiseOrValue { return traceMixed( tracingChannel, - buildResolveCtx(info, args, isTrivialResolver), + buildResolveCtx(info, args, isDefaultResolver), () => resolveFn(source, args, contextValue, info), ); } diff --git a/src/execution/__tests__/resolve-diagnostics-test.ts b/src/execution/__tests__/resolve-diagnostics-test.ts index 75fe68fe35..d3c0dc4f3b 100644 --- a/src/execution/__tests__/resolve-diagnostics-test.ts +++ b/src/execution/__tests__/resolve-diagnostics-test.ts @@ -116,7 +116,7 @@ describe('resolve diagnostics channel', () => { ); }); - it('reports isTrivialResolver based on field.resolve presence', () => { + it('reports isDefaultResolver based on field.resolve presence', () => { const trivialSchema = new GraphQLSchema({ query: new GraphQLObjectType({ name: 'Query', @@ -141,7 +141,7 @@ describe('resolve diagnostics channel', () => { const starts = active.events.filter((e) => e.kind === 'start'); const byField = new Map( - starts.map((e) => [e.ctx.fieldName, e.ctx.isTrivialResolver]), + starts.map((e) => [e.ctx.fieldName, e.ctx.isDefaultResolver]), ); expect(byField.get('trivial')).to.equal(true); expect(byField.get('custom')).to.equal(false); From add27e435b70779b6c6ae31d0ab46cd335487152 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 22:35:43 +0300 Subject: [PATCH 23/29] move helpers into class --- src/execution/Executor.ts | 91 ++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 48 deletions(-) diff --git a/src/execution/Executor.ts b/src/execution/Executor.ts index 276deddcf0..5e9cbfd910 100644 --- a/src/execution/Executor.ts +++ b/src/execution/Executor.ts @@ -626,7 +626,7 @@ export class Executor< // is provided to every resolve function within an execution. It is commonly // used to represent an authenticated user, or request-specific caches. const result = tracingChannel - ? invokeResolverWithTracing( + ? this.invokeResolverWithTracing( tracingChannel, resolveFn, source, @@ -672,6 +672,48 @@ export class Executor< } } + invokeResolverWithTracing( + tracingChannel: MinimalTracingChannel, + resolveFn: GraphQLFieldResolver, + source: unknown, + args: { readonly [argument: string]: unknown }, + contextValue: unknown, + info: GraphQLResolveInfo, + isTrivialResolver: boolean, + ): PromiseOrValue { + return traceMixed( + tracingChannel, + this.buildResolveCtx(args, info, isTrivialResolver), + () => resolveFn(source, args, contextValue, info), + ); + } + + /** + * Build a graphql:resolve channel context for a single field invocation. + * + * `fieldPath` is exposed as a lazy getter because serializing the response + * path is O(depth) and APMs that depth-filter or skip default resolvers + * often never read it. `args` is passed through by reference. + */ + buildResolveCtx( + args: ObjMap, + info: GraphQLResolveInfo, + isDefaultResolver: boolean, + ): object { + let cachedFieldPath: string | undefined; + return { + fieldName: info.fieldName, + parentType: info.parentType.name, + fieldType: String(info.returnType), + args, + isDefaultResolver, + get fieldPath() { + cachedFieldPath ??= pathToArray(info.path).join('.'); + return cachedFieldPath; + }, + }; + } + handleFieldError( rawError: unknown, returnType: GraphQLOutputType, @@ -1418,50 +1460,3 @@ export class Executor< function toNodes(fieldDetailsList: FieldDetailsList): ReadonlyArray { return fieldDetailsList.map((fieldDetails) => fieldDetails.node); } - -/** - * Build a graphql:resolve channel context for a single field invocation. - * - * `fieldPath` is exposed as a lazy getter because serializing the response - * path is O(depth) and APMs that depth-filter or skip trivial resolvers - * often never read it. `args` is passed through by reference. - */ -function buildResolveCtx( - info: GraphQLResolveInfo, - args: { readonly [argument: string]: unknown }, - isDefaultResolver: boolean, -): object { - let cachedFieldPath: string | undefined; - return { - fieldName: info.fieldName, - parentType: info.parentType.name, - fieldType: String(info.returnType), - args, - isDefaultResolver, - get fieldPath() { - cachedFieldPath ??= pathToArray(info.path).join('.'); - return cachedFieldPath; - }, - }; -} - -/** - * Traced path for a single resolver call. Extracted as a module-scope function to increase likelihood of inlining. - * - * @internal - */ -function invokeResolverWithTracing( - tracingChannel: MinimalTracingChannel, - resolveFn: GraphQLFieldResolver, - source: unknown, - args: { readonly [argument: string]: unknown }, - contextValue: unknown, - info: GraphQLResolveInfo, - isDefaultResolver: boolean, -): PromiseOrValue { - return traceMixed( - tracingChannel, - buildResolveCtx(info, args, isDefaultResolver), - () => resolveFn(source, args, contextValue, info), - ); -} From ba734593ad7cf2edcb36865b08a70371d91435aa Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 22:37:47 +0300 Subject: [PATCH 24/29] condense Co-authored-by: Copilot --- src/diagnostics.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/diagnostics.ts b/src/diagnostics.ts index de11c758cf..b9516eedba 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -181,6 +181,5 @@ export function traceMixed( export function shouldTrace( channel: MinimalTracingChannel | undefined, ): channel is MinimalTracingChannel { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare - return channel !== undefined && channel.hasSubscribers !== false; + return channel?.hasSubscribers === true; } From e0ecc566a2035df29f284000330f0c10c6ae11a4 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 22:51:10 +0300 Subject: [PATCH 25/29] move subscription event tracing to mapSourceToResponse to cover custom perEventExecutors --- src/execution/execute.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 9ce5bb54b0..397d0ac856 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -268,14 +268,7 @@ export function executeSync(args: ExecutionArgs): ExecutionResult { export function executeSubscriptionEvent( validatedExecutionArgs: ValidatedExecutionArgs, ): PromiseOrValue { - if (!executeChannel?.hasSubscribers) { - return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); - } - return traceMixed( - executeChannel, - buildExecuteCtxFromValidatedArgs(validatedExecutionArgs), - () => executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs), - ); + return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs); } /** @@ -624,7 +617,14 @@ function mapSourceToResponse( ...validatedExecutionArgs, rootValue: payload, }; - return validatedExecutionArgs.perEventExecutor(perEventExecutionArgs); + if (!executeChannel?.hasSubscribers) { + return validatedExecutionArgs.perEventExecutor(perEventExecutionArgs); + } + return traceMixed( + executeChannel, + buildExecuteCtxFromValidatedArgs(validatedExecutionArgs), + () => validatedExecutionArgs.perEventExecutor(perEventExecutionArgs), + ); } const externalAbortSignal = validatedExecutionArgs.externalAbortSignal; From 0a441054b7b3ac72793ca89f56b55c7f4a2a8c4f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 23:05:14 +0300 Subject: [PATCH 26/29] fixes failing test --- .../__tests__/execute-diagnostics-test.ts | 60 ++++++++++++------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/src/execution/__tests__/execute-diagnostics-test.ts b/src/execution/__tests__/execute-diagnostics-test.ts index 896189c7a3..c1dfdcfc2e 100644 --- a/src/execution/__tests__/execute-diagnostics-test.ts +++ b/src/execution/__tests__/execute-diagnostics-test.ts @@ -1,4 +1,4 @@ -import { expect } from 'chai'; +import { assert, expect } from 'chai'; import { afterEach, describe, it } from 'mocha'; import { @@ -6,17 +6,17 @@ import { getTracingChannel, } from '../../__testUtils__/diagnosticsTestUtils.js'; +import { isAsyncIterable } from '../../jsutils/isAsyncIterable.js'; + import { parse } from '../../language/parser.js'; import { buildSchema } from '../../utilities/buildASTSchema.js'; -import type { ExecutionArgs } from '../execute.js'; import { execute, executeIgnoringIncremental, - executeSubscriptionEvent, executeSync, - validateExecutionArgs, + subscribe, } from '../execute.js'; const schema = buildSchema(` @@ -109,30 +109,50 @@ describe('execute diagnostics channel', () => { ]); }); - it('emits for each executeSubscriptionEvent call with resolved operation ctx', () => { - const args: ExecutionArgs = { - schema, - document: parse('query Q { sync }'), - rootValue, - }; - const validated = validateExecutionArgs(args); - if (!('schema' in validated)) { - throw new Error('unexpected validation failure'); + it('emits for each subscription event with resolved operation ctx', async () => { + const subscriptionSchema = buildSchema(` + type Query { + dummy: String + } + + type Subscription { + tick: String + } + `); + + async function* tickGenerator() { + await Promise.resolve(); + yield { tick: 'one' }; + yield { tick: 'two' }; } + const document = parse('subscription S { tick }'); + active = collectEvents(executeChannel); - // eslint-disable-next-line @typescript-eslint/no-floating-promises - executeSubscriptionEvent(validated); - // eslint-disable-next-line @typescript-eslint/no-floating-promises - executeSubscriptionEvent(validated); + const subscription = await subscribe({ + schema: subscriptionSchema, + document, + rootValue: { tick: tickGenerator }, + }); + assert(isAsyncIterable(subscription)); + + expect(await subscription.next()).to.deep.equal({ + done: false, + value: { data: { tick: 'one' } }, + }); + expect(await subscription.next()).to.deep.equal({ + done: false, + value: { data: { tick: 'two' } }, + }); const starts = active.events.filter((e) => e.kind === 'start'); expect(starts.length).to.equal(2); for (const ev of starts) { - expect(ev.ctx.operationType).to.equal('query'); - expect(ev.ctx.operationName).to.equal('Q'); - expect(ev.ctx.schema).to.equal(schema); + expect(ev.ctx.operationType).to.equal('subscription'); + expect(ev.ctx.operationName).to.equal('S'); + expect(ev.ctx.operation).to.equal(document.definitions[0]); + expect(ev.ctx.schema).to.equal(subscriptionSchema); } }); From 08d4f3d22090dfe76ed8675b537522865b134d78 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 23:18:01 +0300 Subject: [PATCH 27/29] rename test files also combine execute/subscribe --- .../__tests__/subscribe-diagnostics-test.ts | 123 ------------------ ...te-diagnostics-test.ts => tracing-test.ts} | 121 ++++++++++++++--- ...er-diagnostics-test.ts => tracing-test.ts} | 0 ...te-diagnostics-test.ts => tracing-test.ts} | 0 4 files changed, 102 insertions(+), 142 deletions(-) delete mode 100644 src/execution/__tests__/subscribe-diagnostics-test.ts rename src/execution/__tests__/{execute-diagnostics-test.ts => tracing-test.ts} (59%) rename src/language/__tests__/{parser-diagnostics-test.ts => tracing-test.ts} (100%) rename src/validation/__tests__/{validate-diagnostics-test.ts => tracing-test.ts} (100%) diff --git a/src/execution/__tests__/subscribe-diagnostics-test.ts b/src/execution/__tests__/subscribe-diagnostics-test.ts deleted file mode 100644 index 7ea78d6836..0000000000 --- a/src/execution/__tests__/subscribe-diagnostics-test.ts +++ /dev/null @@ -1,123 +0,0 @@ -import { expect } from 'chai'; -import { afterEach, describe, it } from 'mocha'; - -import { - collectEvents, - getTracingChannel, -} from '../../__testUtils__/diagnosticsTestUtils.js'; - -import { isPromise } from '../../jsutils/isPromise.js'; - -import { parse } from '../../language/parser.js'; - -import { GraphQLObjectType } from '../../type/definition.js'; -import { GraphQLString } from '../../type/scalars.js'; -import { GraphQLSchema } from '../../type/schema.js'; - -import { subscribe } from '../execute.js'; - -function buildSubscriptionSchema( - subscribeFn: () => AsyncIterable<{ tick: string }>, -): GraphQLSchema { - return new GraphQLSchema({ - query: new GraphQLObjectType({ - name: 'Query', - fields: { dummy: { type: GraphQLString } }, - }), - subscription: new GraphQLObjectType({ - name: 'Subscription', - fields: { - tick: { - type: GraphQLString, - subscribe: subscribeFn, - }, - }, - }), - }); -} - -async function* twoTicks(): AsyncIterable<{ tick: string }> { - await Promise.resolve(); - yield { tick: 'one' }; - yield { tick: 'two' }; -} - -const subscribeChannel = getTracingChannel('graphql:subscribe'); - -describe('subscribe diagnostics channel', () => { - let active: ReturnType | undefined; - - afterEach(() => { - active?.unsubscribe(); - active = undefined; - }); - - it('emits start and end for a synchronous subscription setup', async () => { - active = collectEvents(subscribeChannel); - - const schema = buildSubscriptionSchema(twoTicks); - const document = parse('subscription S { tick }'); - - const result = subscribe({ schema, document }); - const resolved = isPromise(result) ? await result : result; - if (!(Symbol.asyncIterator in resolved)) { - throw new Error('Expected an async iterator'); - } - await resolved.return?.(); - - expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); - expect(active.events[0].ctx.operationType).to.equal('subscription'); - expect(active.events[0].ctx.operationName).to.equal('S'); - expect(active.events[0].ctx.document).to.equal(document); - expect(active.events[0].ctx.schema).to.equal(schema); - }); - - it('emits the full async lifecycle when subscribe resolver returns a promise', async () => { - active = collectEvents(subscribeChannel); - - const asyncResolver = (): Promise> => - Promise.resolve(twoTicks()); - const schema = buildSubscriptionSchema( - asyncResolver as unknown as () => AsyncIterable<{ tick: string }>, - ); - const document = parse('subscription { tick }'); - - const result = subscribe({ schema, document }); - const resolved = isPromise(result) ? await result : result; - if (!(Symbol.asyncIterator in resolved)) { - throw new Error('Expected an async iterator'); - } - await resolved.return?.(); - - expect(active.events.map((e) => e.kind)).to.deep.equal([ - 'start', - 'end', - 'asyncStart', - 'asyncEnd', - ]); - }); - - it('emits only start and end for a synchronous validation failure', () => { - active = collectEvents(subscribeChannel); - - const schema = buildSubscriptionSchema(twoTicks); - // Invalid: no operation. - const document = parse('fragment F on Subscription { tick }'); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - subscribe({ schema, document }); - - expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); - }); - - it('does nothing when no subscribers are attached', async () => { - const schema = buildSubscriptionSchema(twoTicks); - const document = parse('subscription { tick }'); - - const result = subscribe({ schema, document }); - const resolved = isPromise(result) ? await result : result; - if (Symbol.asyncIterator in resolved) { - await resolved.return?.(); - } - }); -}); diff --git a/src/execution/__tests__/execute-diagnostics-test.ts b/src/execution/__tests__/tracing-test.ts similarity index 59% rename from src/execution/__tests__/execute-diagnostics-test.ts rename to src/execution/__tests__/tracing-test.ts index c1dfdcfc2e..b91d252034 100644 --- a/src/execution/__tests__/execute-diagnostics-test.ts +++ b/src/execution/__tests__/tracing-test.ts @@ -7,6 +7,7 @@ import { } from '../../__testUtils__/diagnosticsTestUtils.js'; import { isAsyncIterable } from '../../jsutils/isAsyncIterable.js'; +import { isPromise } from '../../jsutils/isPromise.js'; import { parse } from '../../language/parser.js'; @@ -23,18 +24,22 @@ const schema = buildSchema(` type Query { sync: String async: String + dummy: String } -`); - -const rootValue = { - sync: () => 'hello', - async: () => Promise.resolve('hello-async'), -}; -const executeChannel = getTracingChannel('graphql:execute'); + type Subscription { + tick: String + } +`); describe('execute diagnostics channel', () => { let active: ReturnType | undefined; + const executeChannel = getTracingChannel('graphql:execute'); + + const rootValue = { + sync: () => 'hello', + async: () => Promise.resolve('hello-async'), + }; afterEach(() => { active?.unsubscribe(); @@ -110,16 +115,6 @@ describe('execute diagnostics channel', () => { }); it('emits for each subscription event with resolved operation ctx', async () => { - const subscriptionSchema = buildSchema(` - type Query { - dummy: String - } - - type Subscription { - tick: String - } - `); - async function* tickGenerator() { await Promise.resolve(); yield { tick: 'one' }; @@ -131,7 +126,7 @@ describe('execute diagnostics channel', () => { active = collectEvents(executeChannel); const subscription = await subscribe({ - schema: subscriptionSchema, + schema, document, rootValue: { tick: tickGenerator }, }); @@ -152,7 +147,7 @@ describe('execute diagnostics channel', () => { expect(ev.ctx.operationType).to.equal('subscription'); expect(ev.ctx.operationName).to.equal('S'); expect(ev.ctx.operation).to.equal(document.definitions[0]); - expect(ev.ctx.schema).to.equal(subscriptionSchema); + expect(ev.ctx.schema).to.equal(schema); } }); @@ -162,3 +157,91 @@ describe('execute diagnostics channel', () => { expect(result).to.deep.equal({ data: { sync: 'hello' } }); }); }); + +describe('subscribe diagnostics channel', () => { + let active: ReturnType | undefined; + const subscribeChannel = getTracingChannel('graphql:subscribe'); + + async function* twoTicks(): AsyncIterable<{ tick: string }> { + await Promise.resolve(); + yield { tick: 'one' }; + yield { tick: 'two' }; + } + + afterEach(() => { + active?.unsubscribe(); + active = undefined; + }); + + it('emits start and end for a synchronous subscription setup', async () => { + active = collectEvents(subscribeChannel); + + const document = parse('subscription S { tick }'); + + const result = subscribe({ + schema, + document, + rootValue: { tick: twoTicks }, + }); + const resolved = isPromise(result) ? await result : result; + assert(isAsyncIterable(resolved)); + await resolved.return?.(); + + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + expect(active.events[0].ctx.operationType).to.equal('subscription'); + expect(active.events[0].ctx.operationName).to.equal('S'); + expect(active.events[0].ctx.document).to.equal(document); + expect(active.events[0].ctx.schema).to.equal(schema); + }); + + it('emits the full async lifecycle when subscribe resolver returns a promise', async () => { + active = collectEvents(subscribeChannel); + + const document = parse('subscription { tick }'); + + const result = subscribe({ + schema, + document, + rootValue: { + tick: (): Promise> => + Promise.resolve(twoTicks()), + }, + }); + const resolved = isPromise(result) ? await result : result; + assert(isAsyncIterable(resolved)); + await resolved.return?.(); + + expect(active.events.map((e) => e.kind)).to.deep.equal([ + 'start', + 'end', + 'asyncStart', + 'asyncEnd', + ]); + }); + + it('emits only start and end for a synchronous validation failure', () => { + active = collectEvents(subscribeChannel); + + // Invalid: no operation. + const document = parse('fragment F on Subscription { tick }'); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + subscribe({ schema, document, rootValue: { tick: twoTicks } }); + + expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); + }); + + it('does nothing when no subscribers are attached', async () => { + const document = parse('subscription { tick }'); + + const result = subscribe({ + schema, + document, + rootValue: { tick: twoTicks }, + }); + const resolved = isPromise(result) ? await result : result; + if (isAsyncIterable(resolved)) { + await resolved.return?.(); + } + }); +}); diff --git a/src/language/__tests__/parser-diagnostics-test.ts b/src/language/__tests__/tracing-test.ts similarity index 100% rename from src/language/__tests__/parser-diagnostics-test.ts rename to src/language/__tests__/tracing-test.ts diff --git a/src/validation/__tests__/validate-diagnostics-test.ts b/src/validation/__tests__/tracing-test.ts similarity index 100% rename from src/validation/__tests__/validate-diagnostics-test.ts rename to src/validation/__tests__/tracing-test.ts From a355a1da94ec83592d8b7d56476e87f94604b3d0 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 23:27:38 +0300 Subject: [PATCH 28/29] merge resolve tracing tests into execution tracing file --- .../__tests__/resolve-diagnostics-test.ts | 220 ------------------ src/execution/__tests__/tracing-test.ts | 206 ++++++++++++++++ 2 files changed, 206 insertions(+), 220 deletions(-) delete mode 100644 src/execution/__tests__/resolve-diagnostics-test.ts diff --git a/src/execution/__tests__/resolve-diagnostics-test.ts b/src/execution/__tests__/resolve-diagnostics-test.ts deleted file mode 100644 index d3c0dc4f3b..0000000000 --- a/src/execution/__tests__/resolve-diagnostics-test.ts +++ /dev/null @@ -1,220 +0,0 @@ -import { expect } from 'chai'; -import { afterEach, describe, it } from 'mocha'; - -import { - collectEvents, - getTracingChannel, -} from '../../__testUtils__/diagnosticsTestUtils.js'; - -import { isPromise } from '../../jsutils/isPromise.js'; - -import { parse } from '../../language/parser.js'; - -import { GraphQLObjectType } from '../../type/definition.js'; -import { GraphQLString } from '../../type/scalars.js'; -import { GraphQLSchema } from '../../type/schema.js'; - -import { buildSchema } from '../../utilities/buildASTSchema.js'; - -import { execute } from '../execute.js'; - -const schema = buildSchema(` - type Query { - sync: String - async: String - fail: String - asyncFail: String - plain: String - nested: Nested - } - type Nested { - leaf: String - } -`); - -const rootValue = { - sync: () => 'hello', - async: () => Promise.resolve('hello-async'), - fail: () => { - throw new Error('boom'); - }, - asyncFail: () => Promise.reject(new Error('async-boom')), - // no `plain` resolver, default property-access is used. - plain: 'plain-value', - nested: { leaf: 'leaf-value' }, -}; - -const resolveChannel = getTracingChannel('graphql:resolve'); - -describe('resolve diagnostics channel', () => { - let active: ReturnType | undefined; - - afterEach(() => { - active?.unsubscribe(); - active = undefined; - }); - - it('emits start and end around a synchronous resolver', () => { - active = collectEvents(resolveChannel); - - const result = execute({ schema, document: parse('{ sync }'), rootValue }); - if (isPromise(result)) { - throw new Error('expected sync'); - } - - const starts = active.events.filter((e) => e.kind === 'start'); - expect(starts.length).to.equal(1); - expect(starts[0].ctx.fieldName).to.equal('sync'); - expect(starts[0].ctx.parentType).to.equal('Query'); - expect(starts[0].ctx.fieldType).to.equal('String'); - expect(starts[0].ctx.fieldPath).to.equal('sync'); - - const kinds = active.events.map((e) => e.kind); - expect(kinds).to.deep.equal(['start', 'end']); - }); - - it('emits the full async lifecycle when a resolver returns a promise', async () => { - active = collectEvents(resolveChannel); - - const result = execute({ schema, document: parse('{ async }'), rootValue }); - await result; - - const kinds = active.events.map((e) => e.kind); - expect(kinds).to.deep.equal(['start', 'end', 'asyncStart', 'asyncEnd']); - }); - - it('emits start, error, end when a sync resolver throws', () => { - active = collectEvents(resolveChannel); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - execute({ schema, document: parse('{ fail }'), rootValue }); - - const kinds = active.events.map((e) => e.kind); - expect(kinds).to.deep.equal(['start', 'error', 'end']); - }); - - it('emits full async lifecycle with error when a resolver rejects', async () => { - active = collectEvents(resolveChannel); - - await execute({ - schema, - document: parse('{ asyncFail }'), - rootValue, - }); - - const kinds = active.events.map((e) => e.kind); - expect(kinds).to.deep.equal([ - 'start', - 'end', - 'asyncStart', - 'error', - 'asyncEnd', - ]); - const errorEvent = active.events.find((e) => e.kind === 'error'); - expect((errorEvent?.ctx as { error?: Error }).error?.message).to.equal( - 'async-boom', - ); - }); - - it('reports isDefaultResolver based on field.resolve presence', () => { - const trivialSchema = new GraphQLSchema({ - query: new GraphQLObjectType({ - name: 'Query', - fields: { - trivial: { type: GraphQLString }, - custom: { - type: GraphQLString, - resolve: () => 'explicit', - }, - }, - }), - }); - - active = collectEvents(resolveChannel); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - execute({ - schema: trivialSchema, - document: parse('{ trivial custom }'), - rootValue: { trivial: 'value' }, - }); - - const starts = active.events.filter((e) => e.kind === 'start'); - const byField = new Map( - starts.map((e) => [e.ctx.fieldName, e.ctx.isDefaultResolver]), - ); - expect(byField.get('trivial')).to.equal(true); - expect(byField.get('custom')).to.equal(false); - }); - - it('serializes fieldPath lazily, joining path keys with dots', () => { - active = collectEvents(resolveChannel); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - execute({ - schema, - document: parse('{ nested { leaf } }'), - rootValue, - }); - - const starts = active.events.filter((e) => e.kind === 'start'); - const paths = starts.map((e) => e.ctx.fieldPath); - expect(paths).to.deep.equal(['nested', 'nested.leaf']); - }); - - it('fires once per field, not per schema walk', () => { - active = collectEvents(resolveChannel); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - execute({ - schema, - document: parse('{ sync plain nested { leaf } }'), - rootValue, - }); - - const starts = active.events.filter((e) => e.kind === 'start'); - const endsSync = active.events.filter((e) => e.kind === 'end'); - expect(starts.length).to.equal(4); // sync, plain, nested, nested.leaf - expect(endsSync.length).to.equal(4); - }); - - it('emits per-field for serial mutation execution', async () => { - const mutationSchema = new GraphQLSchema({ - query: new GraphQLObjectType({ - name: 'Query', - fields: { dummy: { type: GraphQLString } }, - }), - mutation: new GraphQLObjectType({ - name: 'Mutation', - fields: { - first: { type: GraphQLString, resolve: () => 'one' }, - second: { type: GraphQLString, resolve: () => 'two' }, - }, - }), - }); - - active = collectEvents(resolveChannel); - - await execute({ - schema: mutationSchema, - document: parse('mutation M { first second }'), - }); - - const starts = active.events.filter((e) => e.kind === 'start'); - expect(starts.map((e) => e.ctx.fieldName)).to.deep.equal([ - 'first', - 'second', - ]); - }); - - it('does nothing when no subscribers are attached', () => { - const result = execute({ - schema, - document: parse('{ sync }'), - rootValue, - }); - if (isPromise(result)) { - throw new Error('expected sync'); - } - }); -}); diff --git a/src/execution/__tests__/tracing-test.ts b/src/execution/__tests__/tracing-test.ts index b91d252034..06a8cb8d47 100644 --- a/src/execution/__tests__/tracing-test.ts +++ b/src/execution/__tests__/tracing-test.ts @@ -11,6 +11,10 @@ import { isPromise } from '../../jsutils/isPromise.js'; import { parse } from '../../language/parser.js'; +import { GraphQLObjectType } from '../../type/definition.js'; +import { GraphQLString } from '../../type/scalars.js'; +import { GraphQLSchema } from '../../type/schema.js'; + import { buildSchema } from '../../utilities/buildASTSchema.js'; import { @@ -24,9 +28,17 @@ const schema = buildSchema(` type Query { sync: String async: String + fail: String + asyncFail: String + plain: String + nested: Nested dummy: String } + type Nested { + leaf: String + } + type Subscription { tick: String } @@ -245,3 +257,197 @@ describe('subscribe diagnostics channel', () => { } }); }); + +describe('resolve diagnostics channel', () => { + let active: ReturnType | undefined; + const resolveChannel = getTracingChannel('graphql:resolve'); + + const rootValue = { + sync: () => 'hello', + async: () => Promise.resolve('hello-async'), + fail: () => { + throw new Error('boom'); + }, + asyncFail: () => Promise.reject(new Error('async-boom')), + // no `plain` resolver, default property-access is used. + plain: 'plain-value', + nested: { leaf: 'leaf-value' }, + }; + + afterEach(() => { + active?.unsubscribe(); + active = undefined; + }); + + it('emits start and end around a synchronous resolver', () => { + active = collectEvents(resolveChannel); + + const result = execute({ + schema, + document: parse('{ sync }'), + rootValue, + }); + if (isPromise(result)) { + throw new Error('expected sync'); + } + + const starts = active.events.filter((e) => e.kind === 'start'); + expect(starts.length).to.equal(1); + expect(starts[0].ctx.fieldName).to.equal('sync'); + expect(starts[0].ctx.parentType).to.equal('Query'); + expect(starts[0].ctx.fieldType).to.equal('String'); + expect(starts[0].ctx.fieldPath).to.equal('sync'); + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal(['start', 'end']); + }); + + it('emits the full async lifecycle when a resolver returns a promise', async () => { + active = collectEvents(resolveChannel); + + const result = execute({ + schema, + document: parse('{ async }'), + rootValue, + }); + await result; + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal(['start', 'end', 'asyncStart', 'asyncEnd']); + }); + + it('emits start, error, end when a sync resolver throws', () => { + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ schema, document: parse('{ fail }'), rootValue }); + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal(['start', 'error', 'end']); + }); + + it('emits full async lifecycle with error when a resolver rejects', async () => { + active = collectEvents(resolveChannel); + + await execute({ + schema, + document: parse('{ asyncFail }'), + rootValue, + }); + + const kinds = active.events.map((e) => e.kind); + expect(kinds).to.deep.equal([ + 'start', + 'end', + 'asyncStart', + 'error', + 'asyncEnd', + ]); + const errorEvent = active.events.find((e) => e.kind === 'error'); + expect((errorEvent?.ctx as { error?: Error }).error?.message).to.equal( + 'async-boom', + ); + }); + + it('reports isDefaultResolver based on field.resolve presence', () => { + const trivialSchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + trivial: { type: GraphQLString }, + custom: { + type: GraphQLString, + resolve: () => 'explicit', + }, + }, + }), + }); + + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ + schema: trivialSchema, + document: parse('{ trivial custom }'), + rootValue: { trivial: 'value' }, + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + const byField = new Map( + starts.map((e) => [e.ctx.fieldName, e.ctx.isDefaultResolver]), + ); + expect(byField.get('trivial')).to.equal(true); + expect(byField.get('custom')).to.equal(false); + }); + + it('serializes fieldPath lazily, joining path keys with dots', () => { + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ + schema, + document: parse('{ nested { leaf } }'), + rootValue, + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + const paths = starts.map((e) => e.ctx.fieldPath); + expect(paths).to.deep.equal(['nested', 'nested.leaf']); + }); + + it('fires once per field, not per schema walk', () => { + active = collectEvents(resolveChannel); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + execute({ + schema, + document: parse('{ sync plain nested { leaf } }'), + rootValue, + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + const endsSync = active.events.filter((e) => e.kind === 'end'); + expect(starts.length).to.equal(4); // sync, plain, nested, nested.leaf + expect(endsSync.length).to.equal(4); + }); + + it('emits per-field for serial mutation execution', async () => { + const mutationSchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { dummy: { type: GraphQLString } }, + }), + mutation: new GraphQLObjectType({ + name: 'Mutation', + fields: { + first: { type: GraphQLString, resolve: () => 'one' }, + second: { type: GraphQLString, resolve: () => 'two' }, + }, + }), + }); + + active = collectEvents(resolveChannel); + + await execute({ + schema: mutationSchema, + document: parse('mutation M { first second }'), + }); + + const starts = active.events.filter((e) => e.kind === 'start'); + expect(starts.map((e) => e.ctx.fieldName)).to.deep.equal([ + 'first', + 'second', + ]); + }); + + it('does nothing when no subscribers are attached', () => { + const result = execute({ + schema, + document: parse('{ sync }'), + rootValue, + }); + if (isPromise(result)) { + throw new Error('expected sync'); + } + }); +}); From e916ff38bac4b3288eeb0c1d76dd435249060868 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 25 Apr 2026 23:30:26 +0300 Subject: [PATCH 29/29] use per-test rootValue in execution tracing tests --- src/execution/__tests__/tracing-test.ts | 105 +++++++++++++----------- 1 file changed, 57 insertions(+), 48 deletions(-) diff --git a/src/execution/__tests__/tracing-test.ts b/src/execution/__tests__/tracing-test.ts index 06a8cb8d47..574c855864 100644 --- a/src/execution/__tests__/tracing-test.ts +++ b/src/execution/__tests__/tracing-test.ts @@ -39,6 +39,11 @@ const schema = buildSchema(` leaf: String } + type Mutation { + first: String + second: String + } + type Subscription { tick: String } @@ -48,11 +53,6 @@ describe('execute diagnostics channel', () => { let active: ReturnType | undefined; const executeChannel = getTracingChannel('graphql:execute'); - const rootValue = { - sync: () => 'hello', - async: () => Promise.resolve('hello-async'), - }; - afterEach(() => { active?.unsubscribe(); active = undefined; @@ -62,7 +62,11 @@ describe('execute diagnostics channel', () => { active = collectEvents(executeChannel); const document = parse('query Q { sync }'); - const result = execute({ schema, document, rootValue }); + const result = execute({ + schema, + document, + rootValue: { sync: () => 'hello' }, + }); expect(result).to.deep.equal({ data: { sync: 'hello' } }); expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); @@ -76,7 +80,11 @@ describe('execute diagnostics channel', () => { active = collectEvents(executeChannel); const document = parse('query { async }'); - const result = await execute({ schema, document, rootValue }); + const result = await execute({ + schema, + document, + rootValue: { async: () => Promise.resolve('hello-async') }, + }); expect(result).to.deep.equal({ data: { async: 'hello-async' } }); expect(active.events.map((e) => e.kind)).to.deep.equal([ @@ -91,7 +99,7 @@ describe('execute diagnostics channel', () => { active = collectEvents(executeChannel); const document = parse('{ sync }'); - executeSync({ schema, document, rootValue }); + executeSync({ schema, document, rootValue: { sync: () => 'hello' } }); expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); }); @@ -101,7 +109,11 @@ describe('execute diagnostics channel', () => { const document = parse('query Q { sync }'); // eslint-disable-next-line @typescript-eslint/no-floating-promises - executeIgnoringIncremental({ schema, document, rootValue }); + executeIgnoringIncremental({ + schema, + document, + rootValue: { sync: () => 'hello' }, + }); expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); expect(active.events[0].ctx.operationName).to.equal('Q'); @@ -115,9 +127,7 @@ describe('execute diagnostics channel', () => { type Query { sync: String } `); const document = parse('{ sync }'); - expect(() => - execute({ schema: schemaWithDefer, document, rootValue }), - ).to.throw(); + expect(() => execute({ schema: schemaWithDefer, document })).to.throw(); expect(active.events.map((e) => e.kind)).to.deep.equal([ 'start', @@ -165,7 +175,11 @@ describe('execute diagnostics channel', () => { it('does nothing when no subscribers are attached', () => { const document = parse('{ sync }'); - const result = execute({ schema, document, rootValue }); + const result = execute({ + schema, + document, + rootValue: { sync: () => 'hello' }, + }); expect(result).to.deep.equal({ data: { sync: 'hello' } }); }); }); @@ -238,7 +252,7 @@ describe('subscribe diagnostics channel', () => { const document = parse('fragment F on Subscription { tick }'); // eslint-disable-next-line @typescript-eslint/no-floating-promises - subscribe({ schema, document, rootValue: { tick: twoTicks } }); + subscribe({ schema, document }); expect(active.events.map((e) => e.kind)).to.deep.equal(['start', 'end']); }); @@ -262,18 +276,6 @@ describe('resolve diagnostics channel', () => { let active: ReturnType | undefined; const resolveChannel = getTracingChannel('graphql:resolve'); - const rootValue = { - sync: () => 'hello', - async: () => Promise.resolve('hello-async'), - fail: () => { - throw new Error('boom'); - }, - asyncFail: () => Promise.reject(new Error('async-boom')), - // no `plain` resolver, default property-access is used. - plain: 'plain-value', - nested: { leaf: 'leaf-value' }, - }; - afterEach(() => { active?.unsubscribe(); active = undefined; @@ -285,7 +287,7 @@ describe('resolve diagnostics channel', () => { const result = execute({ schema, document: parse('{ sync }'), - rootValue, + rootValue: { sync: () => 'hello' }, }); if (isPromise(result)) { throw new Error('expected sync'); @@ -308,7 +310,7 @@ describe('resolve diagnostics channel', () => { const result = execute({ schema, document: parse('{ async }'), - rootValue, + rootValue: { async: () => Promise.resolve('hello-async') }, }); await result; @@ -320,7 +322,15 @@ describe('resolve diagnostics channel', () => { active = collectEvents(resolveChannel); // eslint-disable-next-line @typescript-eslint/no-floating-promises - execute({ schema, document: parse('{ fail }'), rootValue }); + execute({ + schema, + document: parse('{ fail }'), + rootValue: { + fail: () => { + throw new Error('boom'); + }, + }, + }); const kinds = active.events.map((e) => e.kind); expect(kinds).to.deep.equal(['start', 'error', 'end']); @@ -332,7 +342,9 @@ describe('resolve diagnostics channel', () => { await execute({ schema, document: parse('{ asyncFail }'), - rootValue, + rootValue: { + asyncFail: () => Promise.reject(new Error('async-boom')), + }, }); const kinds = active.events.map((e) => e.kind); @@ -387,7 +399,9 @@ describe('resolve diagnostics channel', () => { execute({ schema, document: parse('{ nested { leaf } }'), - rootValue, + rootValue: { + nested: { leaf: 'leaf-value' }, + }, }); const starts = active.events.filter((e) => e.kind === 'start'); @@ -402,7 +416,12 @@ describe('resolve diagnostics channel', () => { execute({ schema, document: parse('{ sync plain nested { leaf } }'), - rootValue, + rootValue: { + sync: () => 'hello', + // no `plain` resolver, default property-access is used. + plain: 'plain-value', + nested: { leaf: 'leaf-value' }, + }, }); const starts = active.events.filter((e) => e.kind === 'start'); @@ -412,25 +431,15 @@ describe('resolve diagnostics channel', () => { }); it('emits per-field for serial mutation execution', async () => { - const mutationSchema = new GraphQLSchema({ - query: new GraphQLObjectType({ - name: 'Query', - fields: { dummy: { type: GraphQLString } }, - }), - mutation: new GraphQLObjectType({ - name: 'Mutation', - fields: { - first: { type: GraphQLString, resolve: () => 'one' }, - second: { type: GraphQLString, resolve: () => 'two' }, - }, - }), - }); - active = collectEvents(resolveChannel); await execute({ - schema: mutationSchema, + schema, document: parse('mutation M { first second }'), + rootValue: { + first: () => 'one', + second: () => 'two', + }, }); const starts = active.events.filter((e) => e.kind === 'start'); @@ -444,7 +453,7 @@ describe('resolve diagnostics channel', () => { const result = execute({ schema, document: parse('{ sync }'), - rootValue, + rootValue: { sync: () => 'hello' }, }); if (isPromise(result)) { throw new Error('expected sync');