fixups post review

scality · Jul 3, 2024 · ccea8fd · ccea8fd
1 parent e4c98b6
commit ccea8fd
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 16 deletions.
diff --git a/src/CircuitBreaker.ts b/src/CircuitBreaker.ts
@@ -24,14 +24,14 @@ export class CircuitBreaker extends EventEmitter {
     // a promise that unit tests can await on, as jest's fake timers
     // get confused with async flows
     _evaluatingPromiseHook: Promise<unknown> | null;
-    _promError: boolean;
+    _failedProbes: boolean;
 
     constructor(config: unknown) {
         super();
 
         this._config = validate(config);
         this._probes = (this._config.probes || []).map(buildProbe);
-        this._promError = false;
+        this._failedProbes = false;
 
         this._aggregateState = BreakerState.Nominal;
         this._stabilizingCounter = 0;
@@ -51,8 +51,8 @@ export class CircuitBreaker extends EventEmitter {
         return this._aggregateState;
     }
 
-    get promErr(): boolean {
-        return this._promError;
+    get failedProbes(): boolean {
+        return this._failedProbes;
     }
 
     start() {
@@ -68,12 +68,9 @@ export class CircuitBreaker extends EventEmitter {
     }
 
     async _evaluate() {
-        this._promError = false;
-        this._evaluatingPromiseHook = Promise.allSettled(this._probes.map(p => p.check().catch(error => {
-            if (error instanceof Error && error.message === 'Failed to query Prometheus') {
-                this._promError = true;
-            }
-            return 'failed';
+        this._failedProbes = false;
+        this._evaluatingPromiseHook = Promise.allSettled(this._probes.map(p => p.check().catch(() => {
+            this._failedProbes = true;
         })));
         await this._evaluatingPromiseHook;
         this._evaluatingPromiseHook = null;

diff --git a/src/probes/PrometheusClient.ts b/src/probes/PrometheusClient.ts
@@ -37,7 +37,7 @@ export class PrometheusClient {
             `sum(${q})`;
     }
 
-    async instantQuery(): Promise<number | boolean | null> {
+    async instantQuery(): Promise<number | null> {
         try {
             const res = await this.prom.instantQuery(this.q);
 
@@ -50,7 +50,7 @@ export class PrometheusClient {
             return series[0].value.value;
         } catch (error: unknown) {
             log.error('unable to query prometheus', { error });
-            return false;
+            throw new Error('Failed to query Prometheus');
         }
     }
 }
diff --git a/src/probes/PrometheusQueryProbe.ts b/src/probes/PrometheusQueryProbe.ts
@@ -36,16 +36,13 @@ export class PrometheusQueryProbe implements Probe {
 
     async check() {
         const v = await this.prometheusClient.instantQuery();
-        if (v != null && typeof(v) !== 'boolean') {
+        if (v != null) {
             if (Number.isNaN(v)) {
                 log.warn('warning: ignoring received NaN value (interval too long for retention?)');
                 return;
             }
             this.observed = v;
         }
-        if (v === false) {
-            throw new Error('Failed to query Prometheus');
-        }
     }
 
     get value() {