From 61531c515584106501b285949433fb8215dfba7a Mon Sep 17 00:00:00 2001 From: Vaibhav Jain Date: Thu, 29 Jan 2026 10:43:29 -0800 Subject: [PATCH 1/3] Fix TypeError in canonicalize() when user_id/device_id are non-strings --- packages/node/src/assignment/assignment.ts | 5 +++-- packages/node/src/exposure/exposure.ts | 5 +++-- packages/node/src/util/string.ts | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 packages/node/src/util/string.ts diff --git a/packages/node/src/assignment/assignment.ts b/packages/node/src/assignment/assignment.ts index 6bcd162..332686e 100644 --- a/packages/node/src/assignment/assignment.ts +++ b/packages/node/src/assignment/assignment.ts @@ -1,6 +1,7 @@ import { EvaluationVariant } from '@amplitude/experiment-core'; import { ExperimentUser } from '../types/user'; +import { safeStringTrim } from '../util/string'; /** * @deprecated Assignment tracking is deprecated. Use Exposure tracking. @@ -35,11 +36,11 @@ export class Assignment { } public canonicalize(): string { - let canonical = `${this.user.user_id?.trim()} ${this.user.device_id?.trim()} `; + let canonical = `${safeStringTrim(this.user.user_id)} ${safeStringTrim(this.user.device_id)} `; for (const key of Object.keys(this.results).sort()) { const variant = this.results[key]; if (variant?.key) { - canonical += key.trim() + ' ' + variant?.key?.trim() + ' '; + canonical += safeStringTrim(key) + ' ' + safeStringTrim(variant.key) + ' '; } } return canonical; diff --git a/packages/node/src/exposure/exposure.ts b/packages/node/src/exposure/exposure.ts index 8293d6a..9eab3a3 100644 --- a/packages/node/src/exposure/exposure.ts +++ b/packages/node/src/exposure/exposure.ts @@ -1,6 +1,7 @@ import { EvaluationVariant } from '@amplitude/experiment-core'; import { ExperimentUser } from '../types/user'; +import { safeStringTrim } from '../util/string'; export interface ExposureService { track(exposure: Exposure): Promise; @@ -29,11 +30,11 @@ export class Exposure { } public canonicalize(): string { - let canonical = `${this.user.user_id?.trim()} ${this.user.device_id?.trim()} `; + let canonical = `${safeStringTrim(this.user.user_id)} ${safeStringTrim(this.user.device_id)} `; for (const key of Object.keys(this.results).sort()) { const variant = this.results[key]; if (variant?.key) { - canonical += key.trim() + ' ' + variant?.key?.trim() + ' '; + canonical += safeStringTrim(key) + ' ' + safeStringTrim(variant.key) + ' '; } } return canonical; diff --git a/packages/node/src/util/string.ts b/packages/node/src/util/string.ts new file mode 100644 index 0000000..f7786fa --- /dev/null +++ b/packages/node/src/util/string.ts @@ -0,0 +1,15 @@ +/** + * Safely converts a value to a trimmed string. + * Returns empty string for null/undefined values. + * @param value - The value to convert + * @returns Trimmed string representation + */ +export function safeStringTrim(value: unknown): string { + if (value == null) { + return ''; + } + if (typeof value === 'string') { + return value.trim(); + } + return String(value).trim(); +} From 1b553b93f5926804fd100bd36797b9872a3c39bd Mon Sep 17 00:00:00 2001 From: Vaibhav Jain Date: Thu, 29 Jan 2026 10:49:22 -0800 Subject: [PATCH 2/3] fix: lint --- packages/node/src/assignment/assignment.ts | 7 +++++-- packages/node/src/exposure/exposure.ts | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/node/src/assignment/assignment.ts b/packages/node/src/assignment/assignment.ts index 332686e..8944b63 100644 --- a/packages/node/src/assignment/assignment.ts +++ b/packages/node/src/assignment/assignment.ts @@ -36,11 +36,14 @@ export class Assignment { } public canonicalize(): string { - let canonical = `${safeStringTrim(this.user.user_id)} ${safeStringTrim(this.user.device_id)} `; + let canonical = `${safeStringTrim(this.user.user_id)} ${safeStringTrim( + this.user.device_id, + )} `; for (const key of Object.keys(this.results).sort()) { const variant = this.results[key]; if (variant?.key) { - canonical += safeStringTrim(key) + ' ' + safeStringTrim(variant.key) + ' '; + canonical += + safeStringTrim(key) + ' ' + safeStringTrim(variant.key) + ' '; } } return canonical; diff --git a/packages/node/src/exposure/exposure.ts b/packages/node/src/exposure/exposure.ts index 9eab3a3..28b378d 100644 --- a/packages/node/src/exposure/exposure.ts +++ b/packages/node/src/exposure/exposure.ts @@ -30,11 +30,14 @@ export class Exposure { } public canonicalize(): string { - let canonical = `${safeStringTrim(this.user.user_id)} ${safeStringTrim(this.user.device_id)} `; + let canonical = `${safeStringTrim(this.user.user_id)} ${safeStringTrim( + this.user.device_id, + )} `; for (const key of Object.keys(this.results).sort()) { const variant = this.results[key]; if (variant?.key) { - canonical += safeStringTrim(key) + ' ' + safeStringTrim(variant.key) + ' '; + canonical += + safeStringTrim(key) + ' ' + safeStringTrim(variant.key) + ' '; } } return canonical; From 03072ddae69c361e3260f86919133fa0ce8dc220 Mon Sep 17 00:00:00 2001 From: Vaibhav Jain Date: Thu, 29 Jan 2026 13:50:11 -0800 Subject: [PATCH 3/3] add tests --- .../node/test/assignment/assignment.test.ts | 32 +++++++++++++++++++ packages/node/test/exposure/exposure.test.ts | 32 +++++++++++++++++++ packages/node/test/util/string.test.ts | 15 +++++++++ 3 files changed, 79 insertions(+) create mode 100644 packages/node/test/assignment/assignment.test.ts create mode 100644 packages/node/test/exposure/exposure.test.ts create mode 100644 packages/node/test/util/string.test.ts diff --git a/packages/node/test/assignment/assignment.test.ts b/packages/node/test/assignment/assignment.test.ts new file mode 100644 index 0000000..9e064d5 --- /dev/null +++ b/packages/node/test/assignment/assignment.test.ts @@ -0,0 +1,32 @@ +import { Assignment } from 'src/assignment/assignment'; +import { ExperimentUser } from 'src/types/user'; + +describe('Assignment.canonicalize()', () => { + test('canonicalizes with string user_id and device_id', () => { + const user: ExperimentUser = { + user_id: 'user123', + device_id: 'device456', + }; + const results = { + 'flag-key-1': { key: 'on', value: 'on' }, + 'flag-key-2': { key: 'control', value: 'control' }, + }; + const assignment = new Assignment(user, results); + expect(assignment.canonicalize()).toBe( + 'user123 device456 flag-key-1 on flag-key-2 control ', + ); + }); + + test('handles non-string user_id and device_id without throwing', () => { + const user: ExperimentUser = { + user_id: 12345 as any, + device_id: 67890 as any, + }; + const results = { + 'flag-key-1': { key: 999 as any, value: 'on' }, + }; + const assignment = new Assignment(user, results); + expect(() => assignment.canonicalize()).not.toThrow(); + expect(assignment.canonicalize()).toBe('12345 67890 flag-key-1 999 '); + }); +}); diff --git a/packages/node/test/exposure/exposure.test.ts b/packages/node/test/exposure/exposure.test.ts new file mode 100644 index 0000000..fe560a4 --- /dev/null +++ b/packages/node/test/exposure/exposure.test.ts @@ -0,0 +1,32 @@ +import { Exposure } from 'src/exposure/exposure'; +import { ExperimentUser } from 'src/types/user'; + +describe('Exposure.canonicalize()', () => { + test('canonicalizes with string user_id and device_id', () => { + const user: ExperimentUser = { + user_id: 'user123', + device_id: 'device456', + }; + const results = { + 'flag-key-1': { key: 'on', value: 'on' }, + 'flag-key-2': { key: 'control', value: 'control' }, + }; + const exposure = new Exposure(user, results); + expect(exposure.canonicalize()).toBe( + 'user123 device456 flag-key-1 on flag-key-2 control ', + ); + }); + + test('handles non-string user_id and device_id without throwing', () => { + const user: ExperimentUser = { + user_id: 12345 as any, + device_id: 67890 as any, + }; + const results = { + 'flag-key-1': { key: 999 as any, value: 'on' }, + }; + const exposure = new Exposure(user, results); + expect(() => exposure.canonicalize()).not.toThrow(); + expect(exposure.canonicalize()).toBe('12345 67890 flag-key-1 999 '); + }); +}); diff --git a/packages/node/test/util/string.test.ts b/packages/node/test/util/string.test.ts new file mode 100644 index 0000000..29c85b1 --- /dev/null +++ b/packages/node/test/util/string.test.ts @@ -0,0 +1,15 @@ +import { safeStringTrim } from 'src/util/string'; + +describe('safeStringTrim', () => { + test('trims string values', () => { + expect(safeStringTrim(' hello ')).toBe('hello'); + expect(safeStringTrim('test')).toBe('test'); + }); + + test('converts non-string values to trimmed strings', () => { + expect(safeStringTrim(123)).toBe('123'); + expect(safeStringTrim(null)).toBe(''); + expect(safeStringTrim(undefined)).toBe(''); + expect(safeStringTrim(true)).toBe('true'); + }); +});