Skip to content

Commit 2cd8ce6

Browse files
nirinchevnirinchev
authored
[chore] tweak get-device-id (#535)
* [chore] tweak get-device-id Removes isNodeMachineId arguments and simplifies onTimeout * fix lint issues * Use an abort signal, rework error reporting * fix lint
1 parent fb13b4c commit 2cd8ce6

File tree

2 files changed

+116
-132
lines changed

2 files changed

+116
-132
lines changed

‎packages/device-id/src/get-device-id.spec.ts‎

Lines changed: 82 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ describe('getDeviceId', function () {
88

99
const deviceId = await getDeviceId({
1010
getMachineId,
11-
isNodeMachineId: false,
12-
}).value;
11+
});
1312

1413
expect(deviceId).to.be.a('string');
1514
expect(deviceId).to.have.lengthOf(64); // SHA-256 hex digest length
@@ -22,48 +21,46 @@ describe('getDeviceId', function () {
2221

2322
const resultA = await getDeviceId({
2423
getMachineId,
25-
isNodeMachineId: true,
26-
}).value;
24+
});
2725

2826
const resultB = await getDeviceId({
2927
getMachineId: () => Promise.resolve(mockMachineId.toUpperCase()),
30-
isNodeMachineId: true,
31-
}).value;
28+
});
3229

3330
expect(resultA).to.equal(resultB);
3431
});
3532

3633
it('returns "unknown" when machine id is not found', async function () {
3734
const getMachineId = () => Promise.resolve(undefined);
38-
let capturedError: Error | undefined;
35+
let capturedError: [string, Error] | undefined;
3936

4037
const deviceId = await getDeviceId({
4138
getMachineId,
42-
isNodeMachineId: false,
43-
onError: (error) => {
44-
capturedError = error;
39+
onError: (reason, error) => {
40+
capturedError = [reason, error];
4541
},
46-
}).value;
42+
});
4743

4844
expect(deviceId).to.equal('unknown');
49-
expect(capturedError?.message).to.equal('Failed to resolve machine ID');
45+
expect(capturedError?.[0]).to.equal('resolutionError');
46+
expect(capturedError?.[1].message).to.equal('Failed to resolve machine ID');
5047
});
5148

5249
it('returns "unknown" and calls onError when getMachineId throws', async function () {
5350
const error = new Error('Something went wrong');
5451
const getMachineId = () => Promise.reject(error);
55-
let capturedError: Error | undefined;
52+
let capturedError: [string, Error] | undefined;
5653

5754
const result = await getDeviceId({
5855
getMachineId,
59-
isNodeMachineId: false,
60-
onError: (err) => {
61-
capturedError = err;
56+
onError: (reason, err) => {
57+
capturedError = [reason, err];
6258
},
63-
}).value;
59+
});
6460

6561
expect(result).to.equal('unknown');
66-
expect(capturedError).to.equal(error);
62+
expect(capturedError?.[0]).to.equal('resolutionError');
63+
expect(capturedError?.[1]).to.equal(error);
6764
});
6865

6966
it('produces consistent hash for the same machine id', async function () {
@@ -72,86 +69,84 @@ describe('getDeviceId', function () {
7269

7370
const resultA = await getDeviceId({
7471
getMachineId,
75-
isNodeMachineId: false,
76-
}).value;
72+
});
7773

7874
const resultB = await getDeviceId({
7975
getMachineId,
80-
isNodeMachineId: false,
81-
}).value;
76+
});
8277

8378
expect(resultA).to.equal(resultB);
8479
});
8580

86-
it('handles timeout when getting machine id', async function () {
87-
let timeoutId: NodeJS.Timeout;
88-
const getMachineId = () =>
89-
new Promise<string>((resolve) => {
90-
timeoutId = setTimeout(() => resolve('delayed-id'), 10_000);
81+
const fallbackTestCases: {
82+
fallbackValue?: string;
83+
expectedResult: string;
84+
}[] = [
85+
{ expectedResult: 'unknown' },
86+
{ fallbackValue: 'fallback-id', expectedResult: 'fallback-id' },
87+
];
88+
89+
describe('when timed out', function () {
90+
for (const testCase of fallbackTestCases) {
91+
it(`resolves with ${testCase.expectedResult} when fallback value is ${testCase.fallbackValue ?? 'undefined'}`, async function () {
92+
let timeoutId: NodeJS.Timeout;
93+
const getMachineId = () =>
94+
new Promise<string>((resolve) => {
95+
timeoutId = setTimeout(() => resolve('delayed-id'), 10_000);
96+
});
97+
98+
let capturedError: [string, Error] | undefined;
99+
const result = await getDeviceId({
100+
getMachineId,
101+
onError: (reason, error) => {
102+
capturedError = [reason, error];
103+
},
104+
timeout: 5,
105+
fallbackValue: testCase.fallbackValue,
106+
});
107+
108+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
109+
clearTimeout(timeoutId!);
110+
expect(result).to.equal(testCase.expectedResult);
111+
expect(capturedError?.[0]).to.equal('timeout');
112+
expect(capturedError?.[1].message).to.equal(
113+
'Timeout reached after 5 ms',
114+
);
91115
});
92-
93-
let errorCalled = false;
94-
const result = await getDeviceId({
95-
getMachineId,
96-
isNodeMachineId: false,
97-
onError: () => {
98-
errorCalled = true;
99-
},
100-
timeout: 1,
101-
}).value;
102-
103-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
104-
clearTimeout(timeoutId!);
105-
expect(result).to.equal('unknown');
106-
expect(errorCalled).to.equal(false);
107-
});
108-
109-
it('handles external promise resolution', async function () {
110-
let timeoutId: NodeJS.Timeout;
111-
const getMachineId = () =>
112-
new Promise<string>((resolve) => {
113-
timeoutId = setTimeout(() => resolve('delayed-id'), 10_000);
114-
});
115-
116-
const { resolve, value } = getDeviceId({
117-
getMachineId,
118-
isNodeMachineId: false,
119-
});
120-
121-
resolve('external-id');
122-
123-
const result = await value;
124-
125-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
126-
clearTimeout(timeoutId!);
127-
expect(result).to.be.a('string');
128-
expect(result).to.equal('external-id');
129-
expect(result).to.not.equal('unknown');
116+
}
130117
});
131118

132-
it('handles external promise rejection', async function () {
133-
let timeoutId: NodeJS.Timeout;
134-
const getMachineId = () =>
135-
new Promise<string>((resolve) => {
136-
timeoutId = setTimeout(() => resolve('delayed-id'), 10_000);
119+
describe('when aborted', function () {
120+
for (const testCase of fallbackTestCases) {
121+
it(`resolves with ${testCase.expectedResult} when fallback value is ${testCase.fallbackValue ?? 'undefined'}`, async function () {
122+
let timeoutId: NodeJS.Timeout;
123+
const getMachineId = () =>
124+
new Promise<string>((resolve) => {
125+
timeoutId = setTimeout(() => resolve('delayed-id'), 10_000);
126+
});
127+
128+
let capturedError: [string, Error] | undefined;
129+
const abortController = new AbortController();
130+
const value = getDeviceId({
131+
getMachineId,
132+
abortSignal: abortController.signal,
133+
onError: (reason, error) => {
134+
capturedError = [reason, error];
135+
},
136+
fallbackValue: testCase.fallbackValue,
137+
});
138+
139+
abortController.abort();
140+
141+
const result = await value;
142+
143+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
144+
clearTimeout(timeoutId!);
145+
expect(result).to.be.a('string');
146+
expect(result).to.equal(testCase.expectedResult);
147+
expect(capturedError?.[0]).to.equal('abort');
148+
expect(capturedError?.[1].message).to.equal('Aborted by abort signal');
137149
});
138-
139-
const error = new Error('External rejection');
140-
141-
const { reject, value } = getDeviceId({
142-
getMachineId,
143-
isNodeMachineId: false,
144-
});
145-
146-
reject(error);
147-
148-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
149-
clearTimeout(timeoutId!);
150-
try {
151-
await value;
152-
expect.fail('Expected promise to be rejected');
153-
} catch (e) {
154-
expect(e).to.equal(error);
155150
}
156151
});
157152
});

‎packages/device-id/src/get-device-id.ts‎

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,94 +2,83 @@ import { createHmac } from 'crypto';
22

33
export function getDeviceId({
44
getMachineId,
5-
isNodeMachineId,
65
onError,
76
timeout = 3000,
8-
onTimeout,
9-
}: GetDeviceIdOptions): {
10-
/** A promise that resolves to the hashed device ID or `"unknown"` if an error or timeout occurs. */
11-
value: Promise<string>;
12-
/** A function which resolves the device ID promise. */
13-
resolve: (value: string) => void;
14-
/** A function which rejects the device ID promise. */
15-
reject: (err: Error) => void;
16-
} {
17-
let resolveDeviceId!: (value: string) => void;
18-
let rejectDeviceId!: (err: Error) => void;
7+
abortSignal,
8+
fallbackValue = 'unknown',
9+
}: GetDeviceIdOptions): Promise<string> {
1910
let timeoutId: NodeJS.Timeout | undefined;
2011

2112
const value = Promise.race([
2213
resolveMachineId({
2314
getMachineId,
24-
isNodeMachineId,
2515
onError,
2616
}),
27-
new Promise<string>((resolve, reject) => {
17+
new Promise<string>((resolve) => {
18+
abortSignal?.addEventListener('abort', () => {
19+
onError?.('abort', new Error('Aborted by abort signal'));
20+
resolve(fallbackValue);
21+
});
22+
2823
timeoutId = setTimeout(() => {
29-
if (onTimeout) {
30-
onTimeout(resolve, reject);
31-
} else {
32-
resolve('unknown');
33-
}
24+
onError?.('timeout', new Error(`Timeout reached after ${timeout} ms`));
25+
resolve(fallbackValue);
3426
}, timeout).unref?.();
35-
36-
resolveDeviceId = resolve;
37-
rejectDeviceId = reject;
3827
}),
3928
]).finally(() => clearTimeout(timeoutId));
4029

41-
return {
42-
value,
43-
resolve: resolveDeviceId,
44-
reject: rejectDeviceId,
45-
};
30+
return value;
4631
}
4732

4833
export type GetDeviceIdOptions = {
4934
/** A function that returns a raw machine ID. */
5035
getMachineId: () => Promise<string | undefined>;
51-
/** When using node-machine-id, the ID is made uppercase to be consistent with other libraries. */
52-
isNodeMachineId: boolean;
36+
5337
/** Runs when an error occurs while getting the machine ID. */
54-
onError?: (error: Error) => void;
38+
onError?: (
39+
reason: 'abort' | 'timeout' | 'resolutionError',
40+
error: Error,
41+
) => void;
42+
5543
/** Timeout in milliseconds. Defaults to 3000ms. Set to `undefined` to disable. */
5644
timeout?: number | undefined;
57-
/** Runs when the timeout is reached. By default, resolves to "unknown". */
58-
onTimeout?: (
59-
resolve: (value: string) => void,
60-
reject: (err: Error) => void,
61-
) => void;
45+
46+
/**
47+
* An optional abort signal that will cancel the async device ID resolution and will
48+
* cause getDeviceId to resolve immediately with the value of `fallbackValue`.
49+
*/
50+
abortSignal?: AbortSignal;
51+
52+
/**
53+
* An optional fallback value that will be returned if the abort signal is triggered
54+
* or the timeout is reached. Defaults to "unknown".
55+
*/
56+
fallbackValue?: string;
6257
};
6358

6459
async function resolveMachineId({
6560
getMachineId,
66-
isNodeMachineId,
6761
onError,
6862
}: GetDeviceIdOptions): Promise<string> {
6963
try {
70-
const originalId = isNodeMachineId
71-
? (await getMachineId())?.toUpperCase()
72-
: await getMachineId();
64+
const originalId = (await getMachineId())?.toUpperCase();
7365

7466
if (!originalId) {
75-
onError?.(new Error('Failed to resolve machine ID'));
67+
onError?.('resolutionError', new Error('Failed to resolve machine ID'));
7668
return 'unknown';
7769
}
7870

7971
// Create a hashed format from the machine ID
8072
// to match it exactly with the denisbrodbeck/machineid library that Atlas CLI uses.
81-
const hmac = createHmac(
82-
'sha256',
83-
isNodeMachineId ? originalId : originalId,
84-
);
73+
const hmac = createHmac('sha256', originalId);
8574

8675
/** This matches the message used to create the hashes in Atlas CLI */
8776
const DEVICE_ID_HASH_MESSAGE = 'atlascli';
8877

8978
hmac.update(DEVICE_ID_HASH_MESSAGE);
9079
return hmac.digest('hex');
9180
} catch (error) {
92-
onError?.(error as Error);
81+
onError?.('resolutionError', error as Error);
9382
return 'unknown';
9483
}
9584
}

0 commit comments

Comments
 (0)