Skip to content

Commit 11e890d

Browse files
committed
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.
1 parent 8a70ca2 commit 11e890d

8 files changed

Lines changed: 72 additions & 110 deletions

File tree

src/__testUtils__/fakeDiagnosticsChannel.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,14 @@ export class FakeDc implements MinimalDiagnosticsChannel {
123123
}
124124
}
125125

126+
/**
127+
* Shared fake `diagnostics_channel` instance used across all diagnostics
128+
* test suites. `enableDiagnosticsChannel` now throws when called with a
129+
* different `dc` module than the one previously registered, so all test
130+
* files must register the same instance.
131+
*/
132+
export const sharedFakeDc: FakeDc = new FakeDc();
133+
126134
export interface CollectedEvent {
127135
kind: 'start' | 'end' | 'asyncStart' | 'asyncEnd' | 'error';
128136
ctx: { [key: string]: unknown };

src/__tests__/diagnostics-test.ts

Lines changed: 32 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,108 +1,55 @@
11
import { expect } from 'chai';
22
import { describe, it } from 'mocha';
33

4+
import { sharedFakeDc } from '../__testUtils__/fakeDiagnosticsChannel.js';
5+
46
import { invariant } from '../jsutils/invariant.js';
57

6-
import type {
7-
MinimalDiagnosticsChannel,
8-
MinimalTracingChannel,
9-
} from '../diagnostics.js';
108
import { enableDiagnosticsChannel, getChannels } from '../diagnostics.js';
119

12-
function fakeTracingChannel(name: string): MinimalTracingChannel {
13-
const noop: MinimalTracingChannel['start'] = {
14-
hasSubscribers: false,
15-
publish: () => {
16-
/* noop */
17-
},
18-
runStores: <T>(
19-
_ctx: unknown,
20-
fn: (this: unknown, ...args: Array<unknown>) => T,
21-
): T => fn(),
22-
};
23-
const channel: MinimalTracingChannel & { _name: string } = {
24-
_name: name,
25-
hasSubscribers: false,
26-
start: noop,
27-
end: noop,
28-
asyncStart: noop,
29-
asyncEnd: noop,
30-
error: noop,
31-
traceSync: <T>(fn: (...args: Array<unknown>) => T): T => fn(),
32-
};
33-
return channel;
34-
}
35-
36-
function fakeDc(): MinimalDiagnosticsChannel & {
37-
created: Array<string>;
38-
} {
39-
const created: Array<string> = [];
40-
const cache = new Map<string, MinimalTracingChannel>();
41-
return {
42-
created,
43-
tracingChannel(name: string) {
44-
let existing = cache.get(name);
45-
if (existing === undefined) {
46-
created.push(name);
47-
existing = fakeTracingChannel(name);
48-
cache.set(name, existing);
49-
}
50-
return existing;
51-
},
52-
};
53-
}
54-
5510
describe('diagnostics', () => {
56-
it('registers the five graphql tracing channels', () => {
57-
const dc = fakeDc();
58-
enableDiagnosticsChannel(dc);
59-
60-
expect(dc.created).to.deep.equal([
61-
'graphql:execute',
62-
'graphql:parse',
63-
'graphql:validate',
64-
'graphql:resolve',
65-
'graphql:subscribe',
66-
]);
11+
it('exposes the five graphql tracing channels after registration', () => {
12+
enableDiagnosticsChannel(sharedFakeDc);
6713

6814
const channels = getChannels();
6915
invariant(channels !== undefined);
70-
expect(channels.execute).to.not.equal(undefined);
71-
expect(channels.parse).to.not.equal(undefined);
72-
expect(channels.validate).to.not.equal(undefined);
73-
expect(channels.resolve).to.not.equal(undefined);
74-
expect(channels.subscribe).to.not.equal(undefined);
16+
expect(channels.execute).to.equal(
17+
sharedFakeDc.tracingChannel('graphql:execute'),
18+
);
19+
expect(channels.parse).to.equal(
20+
sharedFakeDc.tracingChannel('graphql:parse'),
21+
);
22+
expect(channels.validate).to.equal(
23+
sharedFakeDc.tracingChannel('graphql:validate'),
24+
);
25+
expect(channels.resolve).to.equal(
26+
sharedFakeDc.tracingChannel('graphql:resolve'),
27+
);
28+
expect(channels.subscribe).to.equal(
29+
sharedFakeDc.tracingChannel('graphql:subscribe'),
30+
);
7531
});
7632

77-
it('re-registration with the same module preserves channel identity', () => {
78-
const dc = fakeDc();
79-
enableDiagnosticsChannel(dc);
33+
it('re-registration with the same module is a no-op', () => {
34+
enableDiagnosticsChannel(sharedFakeDc);
8035
const first = getChannels();
8136
invariant(first !== undefined);
8237

83-
enableDiagnosticsChannel(dc);
38+
enableDiagnosticsChannel(sharedFakeDc);
8439
const second = getChannels();
85-
invariant(second !== undefined);
8640

87-
expect(second.execute).to.equal(first.execute);
88-
expect(second.parse).to.equal(first.parse);
89-
expect(second.validate).to.equal(first.validate);
90-
expect(second.resolve).to.equal(first.resolve);
91-
expect(second.subscribe).to.equal(first.subscribe);
41+
expect(second).to.equal(first);
9242
});
9343

94-
it('re-registration with a different module replaces stored references', () => {
95-
const dc1 = fakeDc();
96-
const dc2 = fakeDc();
97-
98-
enableDiagnosticsChannel(dc1);
99-
const first = getChannels();
100-
invariant(first !== undefined);
101-
102-
enableDiagnosticsChannel(dc2);
103-
const second = getChannels();
104-
invariant(second !== undefined);
44+
it('re-registration with a different module throws', () => {
45+
enableDiagnosticsChannel(sharedFakeDc);
10546

106-
expect(second.execute).to.not.equal(first.execute);
47+
expect(() =>
48+
enableDiagnosticsChannel({
49+
tracingChannel: () => {
50+
throw new Error('should not be called');
51+
},
52+
}),
53+
).to.throw(/different `diagnostics_channel` module/);
10754
});
10855
});

src/diagnostics.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export interface GraphQLChannels {
7474
}
7575

7676
let channels: GraphQLChannels | undefined;
77+
let registeredDc: MinimalDiagnosticsChannel | undefined;
7778

7879
/**
7980
* Internal accessor used at emission sites. Returns `undefined` when no
@@ -98,20 +99,31 @@ export function getChannels(): GraphQLChannels | undefined {
9899
* - `graphql:subscribe`
99100
* - `graphql:resolve`
100101
*
101-
* Calling this repeatedly is safe: subsequent calls replace the stored
102-
* channel references, but since `tracingChannel(name)` is cached by name,
103-
* the channel identities remain stable across registrations from the same
104-
* underlying module.
102+
* @throws {Error} If a different `diagnostics_channel` module is registered.
105103
*
106104
* @example
107105
* ```ts
108106
* import dc from 'node:diagnostics_channel';
109107
* import { enableDiagnosticsChannel } from 'graphql';
110108
*
111-
* enableDiagnosticsChannel(dc);
109+
* try {
110+
* enableDiagnosticsChannel(dc);
111+
* } catch {
112+
* // A diagnostic_channel module was already registered, safe to subscribe.
113+
* }
112114
* ```
113115
*/
114116
export function enableDiagnosticsChannel(dc: MinimalDiagnosticsChannel): void {
117+
if (registeredDc !== undefined) {
118+
if (registeredDc !== dc) {
119+
throw new Error(
120+
'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.',
121+
);
122+
}
123+
return;
124+
}
125+
126+
registeredDc = dc;
115127
channels = {
116128
execute: dc.tracingChannel('graphql:execute'),
117129
parse: dc.tracingChannel('graphql:parse'),

src/execution/__tests__/execute-diagnostics-test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha';
33

44
import {
55
collectEvents,
6-
FakeDc,
6+
sharedFakeDc,
77
} from '../../__testUtils__/fakeDiagnosticsChannel.js';
88

99
import { parse } from '../../language/parser.js';
@@ -32,14 +32,13 @@ const rootValue = {
3232
async: () => Promise.resolve('hello-async'),
3333
};
3434

35-
const fakeDc = new FakeDc();
36-
const executeChannel = fakeDc.tracingChannel('graphql:execute');
35+
const executeChannel = sharedFakeDc.tracingChannel('graphql:execute');
3736

3837
describe('execute diagnostics channel', () => {
3938
let active: ReturnType<typeof collectEvents> | undefined;
4039

4140
beforeEach(() => {
42-
enableDiagnosticsChannel(fakeDc);
41+
enableDiagnosticsChannel(sharedFakeDc);
4342
});
4443

4544
afterEach(() => {

src/execution/__tests__/resolve-diagnostics-test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha';
33

44
import {
55
collectEvents,
6-
FakeDc,
6+
sharedFakeDc,
77
} from '../../__testUtils__/fakeDiagnosticsChannel.js';
88

99
import { isPromise } from '../../jsutils/isPromise.js';
@@ -46,14 +46,13 @@ const rootValue = {
4646
nested: { leaf: 'leaf-value' },
4747
};
4848

49-
const fakeDc = new FakeDc();
50-
const resolveChannel = fakeDc.tracingChannel('graphql:resolve');
49+
const resolveChannel = sharedFakeDc.tracingChannel('graphql:resolve');
5150

5251
describe('resolve diagnostics channel', () => {
5352
let active: ReturnType<typeof collectEvents> | undefined;
5453

5554
beforeEach(() => {
56-
enableDiagnosticsChannel(fakeDc);
55+
enableDiagnosticsChannel(sharedFakeDc);
5756
});
5857

5958
afterEach(() => {

src/execution/__tests__/subscribe-diagnostics-test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha';
33

44
import {
55
collectEvents,
6-
FakeDc,
6+
sharedFakeDc,
77
} from '../../__testUtils__/fakeDiagnosticsChannel.js';
88

99
import { isPromise } from '../../jsutils/isPromise.js';
@@ -44,14 +44,13 @@ async function* twoTicks(): AsyncIterable<{ tick: string }> {
4444
yield { tick: 'two' };
4545
}
4646

47-
const fakeDc = new FakeDc();
48-
const subscribeChannel = fakeDc.tracingChannel('graphql:subscribe');
47+
const subscribeChannel = sharedFakeDc.tracingChannel('graphql:subscribe');
4948

5049
describe('subscribe diagnostics channel', () => {
5150
let active: ReturnType<typeof collectEvents> | undefined;
5251

5352
beforeEach(() => {
54-
enableDiagnosticsChannel(fakeDc);
53+
enableDiagnosticsChannel(sharedFakeDc);
5554
});
5655

5756
afterEach(() => {

src/language/__tests__/parser-diagnostics-test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,20 @@ import { afterEach, beforeEach, describe, it } from 'mocha';
33

44
import {
55
collectEvents,
6-
FakeDc,
6+
sharedFakeDc,
77
} from '../../__testUtils__/fakeDiagnosticsChannel.js';
88

99
import { enableDiagnosticsChannel } from '../../diagnostics.js';
1010

1111
import { parse } from '../parser.js';
1212

13-
const fakeDc = new FakeDc();
14-
const parseChannel = fakeDc.tracingChannel('graphql:parse');
13+
const parseChannel = sharedFakeDc.tracingChannel('graphql:parse');
1514

1615
describe('parse diagnostics channel', () => {
1716
let active: ReturnType<typeof collectEvents> | undefined;
1817

1918
beforeEach(() => {
20-
enableDiagnosticsChannel(fakeDc);
19+
enableDiagnosticsChannel(sharedFakeDc);
2120
});
2221

2322
afterEach(() => {

src/validation/__tests__/validate-diagnostics-test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha';
33

44
import {
55
collectEvents,
6-
FakeDc,
6+
sharedFakeDc,
77
} from '../../__testUtils__/fakeDiagnosticsChannel.js';
88

99
import { parse } from '../../language/parser.js';
@@ -20,14 +20,13 @@ const schema = buildSchema(`
2020
}
2121
`);
2222

23-
const fakeDc = new FakeDc();
24-
const validateChannel = fakeDc.tracingChannel('graphql:validate');
23+
const validateChannel = sharedFakeDc.tracingChannel('graphql:validate');
2524

2625
describe('validate diagnostics channel', () => {
2726
let active: ReturnType<typeof collectEvents> | undefined;
2827

2928
beforeEach(() => {
30-
enableDiagnosticsChannel(fakeDc);
29+
enableDiagnosticsChannel(sharedFakeDc);
3130
});
3231

3332
afterEach(() => {

0 commit comments

Comments
 (0)