Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions src/client/eppo-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,67 @@ describe('EppoClient E2E test', () => {
},
};

describe('error encountered', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

let client: EppoClient;
const mockHooks = td.object<IAssignmentHooks>();

beforeAll(() => {
storage.setEntries({ [flagKey]: mockExperimentConfig });
client = new EppoClient(storage);

td.replace(EppoClient.prototype, 'getAssignmentVariation', function () {
throw new Error('So Graceful Error');
});
});

afterAll(() => {
td.reset();
});

it('returns null when graceful failure if error encountered', async () => {
client.setIsGracefulFailureMode(true);

expect(client.getAssignment('subject-identifer', flagKey, {}, mockHooks)).toBeNull();
expect(client.getBoolAssignment('subject-identifer', flagKey, {}, mockHooks)).toBeNull();
expect(
client.getJSONStringAssignment('subject-identifer', flagKey, {}, mockHooks),
).toBeNull();
expect(client.getNumericAssignment('subject-identifer', flagKey, {}, mockHooks)).toBeNull();
expect(
client.getParsedJSONAssignment('subject-identifer', flagKey, {}, mockHooks),
).toBeNull();
expect(client.getStringAssignment('subject-identifer', flagKey, {}, mockHooks)).toBeNull();
});

it('throws error when graceful failure is false', async () => {
client.setIsGracefulFailureMode(false);

expect(() => {
client.getAssignment('subject-identifer', flagKey, {}, mockHooks);
}).toThrow();

expect(() => {
client.getBoolAssignment('subject-identifer', flagKey, {}, mockHooks);
}).toThrow();

expect(() => {
client.getJSONStringAssignment('subject-identifer', flagKey, {}, mockHooks);
}).toThrow();

expect(() => {
client.getParsedJSONAssignment('subject-identifer', flagKey, {}, mockHooks);
}).toThrow();

expect(() => {
client.getNumericAssignment('subject-identifer', flagKey, {}, mockHooks);
}).toThrow();

expect(() => {
client.getStringAssignment('subject-identifer', flagKey, {}, mockHooks);
}).toThrow();
});
});

describe('setLogger', () => {
beforeAll(() => {
storage.setEntries({ [flagKey]: mockExperimentConfig });
Expand Down
135 changes: 86 additions & 49 deletions src/client/eppo-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export interface IEppoClient {
export default class EppoClient implements IEppoClient {
private queuedEvents: IAssignmentEvent[] = [];
private assignmentLogger: IAssignmentLogger | undefined;
private isGracefulFailureMode = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉


constructor(private configurationStore: IConfigurationStore) {}

Expand All @@ -100,10 +101,14 @@ export default class EppoClient implements IEppoClient {
subjectAttributes: Record<string, any> = {},
assignmentHooks?: IAssignmentHooks | undefined,
): string | null {
return (
this.getAssignmentVariation(subjectKey, flagKey, subjectAttributes, assignmentHooks)
.stringValue ?? null
);
try {
return (
this.getAssignmentVariation(subjectKey, flagKey, subjectAttributes, assignmentHooks)
.stringValue ?? null
);
} catch (error) {
return this.rethrowIfNotGraceful(error);
}
}

public getStringAssignment(
Expand All @@ -113,15 +118,19 @@ export default class EppoClient implements IEppoClient {
subjectAttributes: Record<string, any> = {},
assignmentHooks?: IAssignmentHooks | undefined,
): string | null {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.StringType,
).stringValue ?? null
);
try {
return (
this.getAssignmentVariation(
Comment on lines +121 to +123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since all these functions do is immediately delegate to getAssignmentVariation(), I'm thinking it would be cleaner to just have the try-catch in that method and forgo it in these user-facing syntactic sugar wrappers.

subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.StringType,
).stringValue ?? null
);
} catch (error) {
return this.rethrowIfNotGraceful(error);
}
}

getBoolAssignment(
Expand All @@ -131,15 +140,19 @@ export default class EppoClient implements IEppoClient {
subjectAttributes: Record<string, any> = {},
assignmentHooks?: IAssignmentHooks | undefined,
): boolean | null {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.BoolType,
).boolValue ?? null
);
try {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.BoolType,
).boolValue ?? null
);
} catch (error) {
return this.rethrowIfNotGraceful(error);
}
}

getNumericAssignment(
Expand All @@ -148,15 +161,19 @@ export default class EppoClient implements IEppoClient {
subjectAttributes?: Record<string, EppoValue>,
assignmentHooks?: IAssignmentHooks | undefined,
): number | null {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.NumericType,
).numericValue ?? null
);
try {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.NumericType,
).numericValue ?? null
);
} catch (error) {
return this.rethrowIfNotGraceful(error);
}
}

public getJSONStringAssignment(
Expand All @@ -166,15 +183,19 @@ export default class EppoClient implements IEppoClient {
subjectAttributes: Record<string, any> = {},
assignmentHooks?: IAssignmentHooks | undefined,
): string | null {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.JSONType,
).stringValue ?? null
);
try {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.JSONType,
).stringValue ?? null
);
} catch (error) {
return this.rethrowIfNotGraceful(error);
}
}

public getParsedJSONAssignment(
Expand All @@ -184,15 +205,27 @@ export default class EppoClient implements IEppoClient {
subjectAttributes: Record<string, any> = {},
assignmentHooks?: IAssignmentHooks | undefined,
): object | null {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.JSONType,
).objectValue ?? null
);
try {
return (
this.getAssignmentVariation(
subjectKey,
flagKey,
subjectAttributes,
assignmentHooks,
ValueType.JSONType,
).objectValue ?? null
);
} catch (error) {
return this.rethrowIfNotGraceful(error);
}
}

private rethrowIfNotGraceful(err: Error): null {
if (this.isGracefulFailureMode) {
console.error(`[Eppo SDK] Error getting assignment: ${err.message}`);
return null;
}
throw err;
}

private getAssignmentVariation(
Expand Down Expand Up @@ -288,6 +321,10 @@ export default class EppoClient implements IEppoClient {
this.flushQueuedEvents(); // log any events that may have been queued while initializing
}

public setIsGracefulFailureMode(gracefulFailureMode: boolean) {
this.isGracefulFailureMode = gracefulFailureMode;
}
Comment on lines +324 to +326
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea thank you - good idea!


private flushQueuedEvents() {
const eventsToFlush = this.queuedEvents;
this.queuedEvents = [];
Expand Down