Skip to content

Commit 5414635

Browse files
committed
address comments
Signed-off-by: Adam Wootton <[email protected]>
1 parent 40ef268 commit 5414635

File tree

5 files changed

+28
-12
lines changed

5 files changed

+28
-12
lines changed

libs/providers/multi-provider/src/lib/errors.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { ErrorCode, GeneralError, OpenFeatureError } from '@openfeature/server-sdk';
2-
import { PromiseAllSettledResult } from '@opentelemetry/sdk-metrics/build/esnext/utils';
32
import { RegisteredProvider } from './types';
43

54
export class ErrorWithCode extends OpenFeatureError {
@@ -35,7 +34,7 @@ export const constructAggregateError = (providerErrors: { error: unknown; provid
3534
};
3635

3736
export const throwAggregateErrorFromPromiseResults = (
38-
result: PromiseAllSettledResult<unknown>[],
37+
result: PromiseSettledResult<unknown>[],
3938
providerEntries: RegisteredProvider[],
4039
) => {
4140
const errors = result

libs/providers/multi-provider/src/lib/strategies/BaseEvaluationStrategy.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
ErrorCode,
23
EvaluationContext,
34
FlagValue,
45
FlagValueType,
@@ -77,7 +78,11 @@ export abstract class BaseEvaluationStrategy {
7778
resolutions: ProviderResolutionResult<T>[],
7879
): FinalResult<T>;
7980

80-
protected hasError(resolution: ProviderResolutionResult<FlagValue>): boolean {
81+
protected hasError(resolution: ProviderResolutionResult<FlagValue>): resolution is
82+
| ProviderResolutionErrorResult
83+
| (ProviderResolutionSuccessResult<FlagValue> & {
84+
details: ResolutionDetails<FlagValue> & { errorCode: ErrorCode };
85+
}) {
8186
return 'thrownError' in resolution || !!resolution.details.errorCode;
8287
}
8388

libs/providers/multi-provider/src/lib/strategies/ComparisonStrategy.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
ProviderResolutionSuccessResult,
66
StrategyPerProviderContext,
77
} from './BaseEvaluationStrategy';
8-
import { EvaluationContext, FlagValue, Provider } from '@openfeature/server-sdk';
8+
import { EvaluationContext, FlagValue, GeneralError, Provider } from '@openfeature/server-sdk';
99

1010
/**
1111
* Evaluate all providers in parallel and compare the results.
@@ -26,33 +26,45 @@ export class ComparisonStrategy extends BaseEvaluationStrategy {
2626
override determineFinalResult<T extends FlagValue>(
2727
strategyContext: StrategyPerProviderContext,
2828
context: EvaluationContext,
29-
resolutions: ProviderResolutionSuccessResult<T>[],
29+
resolutions: ProviderResolutionResult<T>[],
3030
): FinalResult<T> {
3131
let value: T | undefined;
3232
let fallbackResolution: ProviderResolutionSuccessResult<T> | undefined;
33+
let finalResolution: ProviderResolutionSuccessResult<T> | undefined;
3334
let mismatch = false;
34-
for (const resolution of resolutions) {
35-
if ('thrownError' in resolution || resolution.details.errorCode) {
35+
for (const [i, resolution] of resolutions.entries()) {
36+
if (this.hasError(resolution)) {
3637
return this.collectProviderErrors(resolutions);
3738
}
3839
if (resolution.provider === this.fallbackProvider) {
3940
fallbackResolution = resolution;
4041
}
42+
if (i === 0) {
43+
finalResolution = resolution;
44+
}
4145
if (typeof value !== 'undefined' && value !== resolution.details.value) {
4246
mismatch = true;
4347
} else {
4448
value = resolution.details.value;
4549
}
4650
}
4751

52+
if (!fallbackResolution) {
53+
throw new GeneralError('Fallback provider not found in resolution results');
54+
}
55+
56+
if (!finalResolution) {
57+
throw new GeneralError('Final resolution not found in resolution results');
58+
}
59+
4860
if (mismatch) {
4961
this.onMismatch?.(resolutions);
5062
return {
51-
details: fallbackResolution!.details,
52-
provider: fallbackResolution!.provider,
63+
details: fallbackResolution.details,
64+
provider: fallbackResolution.provider,
5365
};
5466
}
5567

56-
return this.resolutionToFinalResult(resolutions[0]);
68+
return this.resolutionToFinalResult(finalResolution);
5769
}
5870
}

libs/providers/multi-provider/src/lib/strategies/FirstMatchStrategy.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class FirstMatchStrategy extends BaseEvaluationStrategy {
3232
resolutions: ProviderResolutionResult<T>[],
3333
): FinalResult<T> {
3434
const finalResolution = resolutions[resolutions.length - 1];
35-
if ('thrownError' in finalResolution || !!finalResolution.details.errorCode) {
35+
if (this.hasError(finalResolution)) {
3636
return this.collectProviderErrors(resolutions);
3737
}
3838
return this.resolutionToFinalResult(finalResolution);

libs/providers/multi-provider/src/lib/strategies/FirstSuccessfulStrategy.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export class FirstSuccessfulStrategy extends BaseEvaluationStrategy {
2828
resolutions: ProviderResolutionResult<T>[],
2929
): FinalResult<T> {
3030
const finalResolution = resolutions[resolutions.length - 1];
31-
if ('thrownError' in finalResolution || !!finalResolution.details.errorCode) {
31+
if (this.hasError(finalResolution)) {
3232
return this.collectProviderErrors(resolutions);
3333
}
3434
return this.resolutionToFinalResult(finalResolution);

0 commit comments

Comments
 (0)