Skip to content

Commit 935a1b7

Browse files
logaretmyaacovCR
authored andcommitted
support runtimes without TracingChannel.hasSubscribers aggregate
Bun's node:diagnostics_channel ships TracingChannel but does not expose the aggregate hasSubscribers getter, so a subscriber added via tracingChannel.subscribe(handlers) is invisible to the previous ?.hasSubscribers gate and graphql-js silently drops every event. Introduce a shouldTrace helper that trusts the aggregate when present and falls back to checking each of the five underlying lifecycle channels when it is undefined. Route every emission gate (parse/validate/execute/ subscribe/resolve) through it.
1 parent 73a5d6f commit 935a1b7

5 files changed

Lines changed: 43 additions & 12 deletions

File tree

src/diagnostics.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { isPromise } from './jsutils/isPromise.js';
2020
* @internal
2121
*/
2222
export interface MinimalChannel {
23+
readonly hasSubscribers?: boolean;
2324
publish: (message: unknown) => void;
2425
runStores: <T, ContextType extends object>(
2526
context: ContextType,
@@ -37,7 +38,9 @@ export interface MinimalChannel {
3738
* @internal
3839
*/
3940
export interface MinimalTracingChannel {
40-
readonly hasSubscribers: boolean;
41+
// `undefined` accommodates runtimes (e.g. Bun) that ship `tracingChannel`
42+
// without exposing the aggregate `hasSubscribers` getter.
43+
readonly hasSubscribers: boolean | undefined;
4144
readonly start: MinimalChannel;
4245
readonly end: MinimalChannel;
4346
readonly asyncStart: MinimalChannel;
@@ -126,6 +129,33 @@ export const subscribeChannel: MinimalTracingChannel | undefined =
126129
export const resolveChannel: MinimalTracingChannel | undefined =
127130
dc?.tracingChannel('graphql:resolve');
128131

132+
const SUB_CHANNEL_KEYS: ReadonlyArray<
133+
'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error'
134+
> = ['start', 'end', 'asyncStart', 'asyncEnd', 'error'];
135+
136+
/**
137+
* Whether emission sites should publish to `channel`. Trusts the
138+
* `TracingChannel.hasSubscribers` aggregate when the runtime exposes it; if
139+
* the getter is missing (e.g. Bun's `node:diagnostics_channel`, where
140+
* `tracingChannel.hasSubscribers` is `undefined`), falls back to checking
141+
* each of the five underlying lifecycle channels so a subscriber attached
142+
* via `tracingChannel.subscribe(handlers)` is still observed.
143+
*
144+
* @internal
145+
*/
146+
export function shouldTrace(
147+
channel: MinimalTracingChannel | undefined,
148+
): channel is MinimalTracingChannel {
149+
if (channel == null) {
150+
return false;
151+
}
152+
const aggregate = channel.hasSubscribers;
153+
if (aggregate !== undefined) {
154+
return aggregate;
155+
}
156+
return SUB_CHANNEL_KEYS.some((key) => channel[key].hasSubscribers === true);
157+
}
158+
129159
/**
130160
* Publish a mixed sync-or-promise operation through `channel`. Caller has
131161
* already verified that a subscriber is attached.

src/execution/Executor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import {
4444
} from '../type/definition.js';
4545
import type { GraphQLSchema } from '../type/schema.js';
4646

47-
import { resolveChannel, traceMixed } from '../diagnostics.js';
47+
import { resolveChannel, shouldTrace, traceMixed } from '../diagnostics.js';
4848

4949
import { AbortedGraphQLExecutionError } from './AbortedGraphQLExecutionError.js';
5050
import { withCancellation } from './cancellablePromise.js';
@@ -589,7 +589,7 @@ export class Executor<
589589
const returnType = fieldDef.type;
590590
let resolveFn = fieldDef.resolve ?? validatedExecutionArgs.fieldResolver;
591591

592-
if (resolveChannel?.hasSubscribers) {
592+
if (shouldTrace(resolveChannel)) {
593593
const channel = resolveChannel;
594594
const originalResolveFn = resolveFn;
595595
resolveFn = (s, args, c, info) =>

src/execution/execute.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import { getOperationAST } from '../utilities/getOperationAST.js';
3636

3737
import {
3838
executeChannel,
39+
shouldTrace,
3940
subscribeChannel,
4041
traceMixed,
4142
} from '../diagnostics.js';
@@ -74,7 +75,7 @@ const UNEXPECTED_EXPERIMENTAL_DIRECTIVES =
7475
* delivery.
7576
*/
7677
export function execute(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
77-
if (!executeChannel?.hasSubscribers) {
78+
if (!shouldTrace(executeChannel)) {
7879
return executeImpl(args);
7980
}
8081
return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () =>
@@ -138,7 +139,7 @@ function executeImpl(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
138139
export function experimentalExecuteIncrementally(
139140
args: ExecutionArgs,
140141
): PromiseOrValue<ExecutionResult | ExperimentalIncrementalExecutionResults> {
141-
if (!executeChannel?.hasSubscribers) {
142+
if (!shouldTrace(executeChannel)) {
142143
return experimentalExecuteIncrementallyImpl(args);
143144
}
144145
return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () =>
@@ -166,7 +167,7 @@ function experimentalExecuteIncrementallyImpl(
166167
export function executeIgnoringIncremental(
167168
args: ExecutionArgs,
168169
): PromiseOrValue<ExecutionResult | ExperimentalIncrementalExecutionResults> {
169-
if (!executeChannel?.hasSubscribers) {
170+
if (!shouldTrace(executeChannel)) {
170171
return executeIgnoringIncrementalImpl(args);
171172
}
172173
return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () =>
@@ -283,7 +284,7 @@ export function subscribe(
283284
): PromiseOrValue<
284285
AsyncGenerator<ExecutionResult, void, void> | ExecutionResult
285286
> {
286-
if (!subscribeChannel?.hasSubscribers) {
287+
if (!shouldTrace(subscribeChannel)) {
287288
return subscribeImpl(args);
288289
}
289290
return traceMixed(subscribeChannel, buildExecuteCtxFromArgs(args), () =>
@@ -598,7 +599,7 @@ function mapSourceToResponse(
598599
...validatedExecutionArgs,
599600
rootValue: payload,
600601
};
601-
if (!executeChannel?.hasSubscribers) {
602+
if (!shouldTrace(executeChannel)) {
602603
return validatedExecutionArgs.perEventExecutor(perEventExecutionArgs);
603604
}
604605
return traceMixed(

src/language/parser.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { Maybe } from '../jsutils/Maybe.js';
33
import type { GraphQLError } from '../error/GraphQLError.js';
44
import { syntaxError } from '../error/syntaxError.js';
55

6-
import { parseChannel } from '../diagnostics.js';
6+
import { parseChannel, shouldTrace } from '../diagnostics.js';
77

88
import type {
99
ArgumentCoordinateNode,
@@ -134,7 +134,7 @@ export function parse(
134134
source: string | Source,
135135
options?: ParseOptions,
136136
): DocumentNode {
137-
return parseChannel?.hasSubscribers
137+
return shouldTrace(parseChannel)
138138
? parseChannel.traceSync(() => parseImpl(source, options), { source })
139139
: parseImpl(source, options);
140140
}

src/validation/validate.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { assertValidSchema } from '../type/validate.js';
1212

1313
import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js';
1414

15-
import { validateChannel } from '../diagnostics.js';
15+
import { shouldTrace, validateChannel } from '../diagnostics.js';
1616

1717
import { specifiedRules, specifiedSDLRules } from './specifiedRules.js';
1818
import type { SDLValidationRule, ValidationRule } from './ValidationContext.js';
@@ -63,7 +63,7 @@ export function validate(
6363
rules: ReadonlyArray<ValidationRule> = specifiedRules,
6464
options?: ValidationOptions,
6565
): ReadonlyArray<GraphQLError> {
66-
return validateChannel?.hasSubscribers
66+
return shouldTrace(validateChannel)
6767
? validateChannel.traceSync(
6868
() => validateImpl(schema, documentAST, rules, options),
6969
{ schema, document: documentAST },

0 commit comments

Comments
 (0)