From ae6ec233c87edabb0fabe5b1da356c153815065c Mon Sep 17 00:00:00 2001 From: Rohland de Charmoy Date: Thu, 9 Nov 2023 09:44:22 +0200 Subject: [PATCH] :bug: fixed a bunch of bugs relating to error conditions --- package.json | 2 +- src/digest/digest.ts | 14 ++-- src/evaluation.spec.ts | 9 ++- src/evaluators/base.spec.ts | 139 ++++++++++++++++++++++++++++++++++- src/evaluators/base.ts | 65 +++++++++++++++- src/evaluators/mysql.ts | 21 ++---- src/evaluators/sumo.ts | 16 ++-- src/evaluators/web.ts | 15 ++-- src/lib/key.ts | 18 ++++- src/models/channels/slack.ts | 3 +- 10 files changed, 250 insertions(+), 52 deletions(-) diff --git a/package.json b/package.json index 871135e..d0fc163 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "barky", - "version": "1.0.51", + "version": "1.0.52", "description": "A simple cloud services watchdog with digest notification support & no external dependencies", "homepage": "https://github.com/Rohland/barky#readme", "main": "dist/cli.js", diff --git a/src/digest/digest.ts b/src/digest/digest.ts index 4ec5e74..ec50306 100644 --- a/src/digest/digest.ts +++ b/src/digest/digest.ts @@ -6,7 +6,7 @@ import { executeAlerts } from "./alerter"; import { ChannelConfig } from "../models/channels/base"; import { AlertRule, AlertRuleType } from "../models/alert_configuration"; import { DigestConfiguration } from "../models/digest"; -import { findMatchingKeyFor } from "../lib/key"; +import { findMatchingKeyFor, findMatchingKeysFor } from "../lib/key"; import { emitResults } from "../result-emitter"; export async function digest( @@ -142,7 +142,7 @@ export class DigestContext { public addSnapshotForResult(result: Result) { if (result instanceof SkippedResult) { - this.tryFindAndAddExistingSnapshotForSkippedResult(result); + this.tryFindAndAddExistingSnapshotsForSkippedResult(result); return; } const data = { @@ -166,15 +166,15 @@ export class DigestContext { } else { data.date = existingSnapshot?.date ?? data.date; } - this.snapshots.push(new Snapshot(data)); + this._snapshots.push(new Snapshot(data)); } - private tryFindAndAddExistingSnapshotForSkippedResult(result: SkippedResult) { - const found = findMatchingKeyFor(result, Array.from(this._previousSnapshotLookup.values())); - if (!found) { + private tryFindAndAddExistingSnapshotsForSkippedResult(result: SkippedResult) { + const found = findMatchingKeysFor(result, Array.from(this._previousSnapshotLookup.values())); + if (found.length === 0) { return; } - this.snapshots.push(found); + this._snapshots.push(...found); } private generateLogLookup(logs: MonitorLog[]): Map { diff --git a/src/evaluation.spec.ts b/src/evaluation.spec.ts index e2c0a42..721809a 100644 --- a/src/evaluation.spec.ts +++ b/src/evaluation.spec.ts @@ -98,7 +98,10 @@ describe("evaluation", () => { const result = new WebResult(new Date(), "health", "wwww.codeo.co.za", "FAIL", "500", "500", 1, null); const type = new WebEvaluator({}); type.evaluate = jest.fn().mockResolvedValue({ - apps: [{}, {}], + apps: [{ + type: "web", + name: "health" + }], results: [result] }); @@ -113,8 +116,8 @@ describe("evaluation", () => { type: "web", label: "monitor", identifier: "ping", - result: 2, - resultMsg: "2 evaluated", + result: 1, + resultMsg: "1 evaluated", success: true, timeTaken: expect.any(Number), alert: null diff --git a/src/evaluators/base.spec.ts b/src/evaluators/base.spec.ts index 19cd4f9..be16b2b 100644 --- a/src/evaluators/base.spec.ts +++ b/src/evaluators/base.spec.ts @@ -7,6 +7,7 @@ import { SumoEvaluator } from "./sumo"; import { IUniqueKey } from "../lib/key"; import { DefaultTrigger } from "../models/trigger"; import { initLocaleAndTimezone } from "../lib/utility"; +import { MySqlResult, Result } from "../models/result"; describe("base evaluator", () => { @@ -328,6 +329,134 @@ describe("base evaluator", () => { }); }); }); + + describe("evaluateApps", () => { + class MyEval extends BaseEvaluator { + + public skipped: IApp[] = null; + public apps: IApp[] = null; + public results: Result[] = []; + + public addSkipped(skip) { + this.skipped ||= []; + this.skipped.push(skip); + } + + public addApp(app) { + this.apps ||= []; + this.apps.push(app); + } + + public addResult(result) { + this.results.push(result); + } + + configureAndExpandApp(_app: IApp, _name: string): IApp[] { + return []; + } + + public async evaluate(): Promise { + return { + apps: this.apps, + results: this.results, + skippedApps: this.skipped + }; + } + + public generateSkippedAppUniqueKey(name: string): IUniqueKey { + return { + type: "mysql", + label: name, + identifier: "*" + }; + } + + get type(): EvaluatorType { + return undefined; + } + + protected async dispose(): Promise { + return; + } + + protected async tryEvaluate(_app: IApp): Promise { + return []; + } + } + + describe("when results are emitted", () => { + it("should return results and no skipped", async () => { + // arrange + const app = { + type: "mysql", + name: "queue-performance" + }; + + const sut = new MyEval({}); + sut.addApp(app); + const result = new MySqlResult(app.name, "my-queue", "test", "test", 0, true, app); + sut.addResult(result); + + // act + const results = await sut.evaluateApps(); + + // assert + expect(results.skippedApps).toEqual([]); + expect(results.results).toEqual([result]); + }); + }); + + describe("when results are missing (presumably due to underlying error)", () => { + it("should add skipped result to avoid failures resolving", async () => { + // arrange + const app = { + type: "mysql", + name: "queue-performance" + }; + + const sut = new MyEval({}); + sut.addApp(app); + + // act + const results = await sut.evaluateApps(); + + // assert + expect(results.results).toEqual([]); + expect(results.skippedApps.length).toEqual(1); + const skipped = results.skippedApps[0]; + expect(skipped.type).toEqual(app.type); + expect(skipped.label).toEqual(app.name); + expect(skipped.identifier).toEqual("*"); + }); + describe("but if is already skipped", () => { + it("should not add skip again", async () => { + // arrange + const app = { + type: "mysql", + name: "queue-performance" + }; + + const sut = new MyEval({}); + sut.addApp(app); + sut.addSkipped({ + ...app, + ...sut.generateSkippedAppUniqueKey(app.name) + }); + + // act + const results = await sut.evaluateApps(); + + // assert + expect(results.results).toEqual([]); + expect(results.skippedApps.length).toEqual(1); + const skipped = results.skippedApps[0]; + expect(skipped.type).toEqual(app.type); + expect(skipped.label).toEqual(app.name); + expect(skipped.identifier).toEqual("*"); + }); + }); + }); + }); }); class CustomEvaluator extends BaseEvaluator { @@ -336,7 +465,7 @@ class CustomEvaluator extends BaseEvaluator { super(config); } - configureAndExpandApp(app: IApp, name: string): IApp[] { + configureAndExpandApp(app: IApp, _name: string): IApp[] { return [app]; } @@ -356,4 +485,12 @@ class CustomEvaluator extends BaseEvaluator { }; } + protected async dispose(): Promise { + return; + } + + protected tryEvaluate(_app: IApp): Promise { + return Promise.resolve(undefined); + } + } diff --git a/src/evaluators/base.ts b/src/evaluators/base.ts index aeedd25..bf43b41 100644 --- a/src/evaluators/base.ts +++ b/src/evaluators/base.ts @@ -7,6 +7,7 @@ import { LoopMs } from "../loop"; import { IUniqueKey } from "../lib/key"; import { DefaultTrigger, IRule } from "../models/trigger"; import { DayAndTimeEvaluator } from "../lib/time"; +import { MonitorFailureResult, Result } from "../models/result"; const executionCounter = new Map(); @@ -30,15 +31,71 @@ export abstract class BaseEvaluator { } public async evaluateApps(): Promise { - const results = await this.evaluate(); - results.skippedApps = this._skippedApps; - return results; + try { + const results = await this.evaluate(); + results.skippedApps ||= []; + results.skippedApps.push(...(this.skippedApps || [])); + const appResults = results.results; + const apps = results.apps; + apps.forEach(app => { + const hasResultOrWasSkipped = + appResults.find(x => x.label === app.name) + || results.skippedApps.find(x => x.name === app.name); + if (!hasResultOrWasSkipped) { + log(`No result found for app ${ app.name }`); + results.skippedApps.push({ + ...app, + ...this.generateSkippedAppUniqueKey(app.name) + }); + } + }); + return results; + } finally { + if (typeof this.dispose === "function") { + await this.dispose(); + } + } } - protected abstract evaluate(): Promise; + public async evaluate(): Promise { + const apps = this.getAppsToEvaluate(); + const results = await Promise.allSettled(apps.map(async app => { + try { + return await this.tryEvaluate(app) + } catch(err) { + try { + const errorInfo = new Error(err.message); + errorInfo.stack = err.stack; + // @ts-ignore + errorInfo.response = { + status: err?.response?.status, + data: err?.response.data + }; + log(`error executing ${ app.type } evaluator for '${ app.name }': ${ err.message }`, errorInfo); + } catch { + // no-op + } + return new MonitorFailureResult( + app.type, + app.name, + err.message, + app); + } + })); + // see above - we don't expect any failures, as we catch errors + const values = results.map(x => x.status === "fulfilled" ? x.value : null).filter(x => !!x); + return { + results: values.flat(), + apps + }; + } + + protected abstract tryEvaluate(app: IApp): Promise; protected abstract generateSkippedAppUniqueKey(name: string): IUniqueKey; + protected abstract dispose(): Promise; + abstract configureAndExpandApp(app: IApp, name: string): IApp[]; abstract get type(): EvaluatorType; diff --git a/src/evaluators/mysql.ts b/src/evaluators/mysql.ts index 94be077..d02ad17 100644 --- a/src/evaluators/mysql.ts +++ b/src/evaluators/mysql.ts @@ -3,11 +3,9 @@ import { startClock, stopClock } from "../lib/profiler"; import { renderTemplate } from "../lib/renderer"; import { MonitorFailureResult, MySqlResult, Result } from "../models/result"; import { log } from "../models/logger"; -import { EvaluatorResult } from "./types"; import { getAppVariations, IApp } from "../models/app"; import { BaseEvaluator, EvaluatorType, findTriggerRulesFor } from "./base"; import { IUniqueKey } from "../lib/key"; -import { flatten } from "../lib/utility"; import { IRule } from "../models/trigger"; export class MySqlEvaluator extends BaseEvaluator { @@ -29,17 +27,8 @@ export class MySqlEvaluator extends BaseEvaluator { }); } - public async evaluate(): Promise { - const apps = this.getAppsToEvaluate(); - try { - const results = flatten(await Promise.all(apps.map(app => tryEvaluate(app)))); - return { - results, - apps - }; - } finally { - await disposeConnections(); - } + async tryEvaluate(app: IApp) { + return await tryEvaluate(app); } protected generateSkippedAppUniqueKey(name: string): IUniqueKey { @@ -49,9 +38,13 @@ export class MySqlEvaluator extends BaseEvaluator { identifier: "*" // when skipped, we want to match all identifiers under the type:label }; } + + protected async dispose(): Promise { + await disposeConnections(); + } } -async function tryEvaluate(app): Promise { +async function tryEvaluate(app: IApp): Promise { try { const connection = await getConnection(app); const timer = startClock(); diff --git a/src/evaluators/sumo.ts b/src/evaluators/sumo.ts index 0b36f53..74d72ac 100644 --- a/src/evaluators/sumo.ts +++ b/src/evaluators/sumo.ts @@ -5,7 +5,6 @@ import { MonitorFailureResult, SumoResult } from "../models/result"; import { startClock, stopClock } from "../lib/profiler"; import { renderTemplate } from "../lib/renderer"; import { log } from "../models/logger"; -import { EvaluatorResult } from "./types"; import { getAppVariations, IApp } from "../models/app"; import { BaseEvaluator, EvaluatorType, findTriggerRulesFor } from "./base"; import { IUniqueKey } from "../lib/key"; @@ -34,13 +33,8 @@ export class SumoEvaluator extends BaseEvaluator { }); } - async evaluate(): Promise { - const apps = this.getAppsToEvaluate(); - const results = await Promise.all(apps.map(app => tryEvaluate(app))); - return { - results, - apps - }; + async tryEvaluate(app: IApp) { + return await tryEvaluate(app); } protected generateSkippedAppUniqueKey(name: string): IUniqueKey { @@ -50,9 +44,13 @@ export class SumoEvaluator extends BaseEvaluator { identifier: "*" // when skipped, we want to match all identifiers under the type:label }; } + + protected async dispose(): Promise { + return; + } } -async function tryEvaluate(app) { +async function tryEvaluate(app: IApp) { try { const timer = startClock(); app.jobId = await startSearch(app, log); diff --git a/src/evaluators/web.ts b/src/evaluators/web.ts index 40bddf5..df6f00d 100644 --- a/src/evaluators/web.ts +++ b/src/evaluators/web.ts @@ -2,7 +2,6 @@ import axios from "axios"; import { startClock, stopClock } from "../lib/profiler"; import { MonitorFailureResult, WebResult } from "../models/result"; import { log } from "../models/logger"; -import { EvaluatorResult } from "./types"; import { getAppVariations, IApp } from "../models/app"; import { BaseEvaluator, EvaluatorType } from "./base"; import { IUniqueKey } from "../lib/key"; @@ -16,13 +15,8 @@ export class WebEvaluator extends BaseEvaluator { return EvaluatorType.web; } - async evaluate(): Promise { - const apps = this.getAppsToEvaluate(); - const results = await Promise.all(apps.map(app => tryEvaluate(app))); - return { - results, - apps - }; + async tryEvaluate(app: IApp) { + return await tryEvaluate(app); } configureAndExpandApp(app: IApp, name: string): IApp[] { @@ -41,6 +35,11 @@ export class WebEvaluator extends BaseEvaluator { identifier: name }; } + + + protected async dispose(): Promise { + return; + } } async function tryEvaluate(app) { diff --git a/src/lib/key.ts b/src/lib/key.ts index d0026e3..40e87b1 100644 --- a/src/lib/key.ts +++ b/src/lib/key.ts @@ -17,12 +17,24 @@ export function explodeUniqueKey(key: string): IUniqueKey { }; } +function doesKeyMatch(x: T, keyToFind: IUniqueKey) { + return x.type === keyToFind.type + && (x.label === keyToFind.label || x.label === "*" || keyToFind.label === "*") + && (x.identifier === keyToFind.identifier || x.identifier === "*" || keyToFind.identifier === "*"); +} + export function findMatchingKeyFor( keyToFind: IUniqueKey, results: T[]): T { return results.find(x => { - return x.type === keyToFind.type - && (x.label === keyToFind.label || x.label === "*" || keyToFind.label === "*") - && (x.identifier === keyToFind.identifier || x.identifier === "*" || keyToFind.identifier === "*"); + return doesKeyMatch(x, keyToFind); + }); +} + +export function findMatchingKeysFor( + keyToFind: IUniqueKey, + results: T[]): T[] { + return results.filter(x => { + return doesKeyMatch(x, keyToFind); }); } diff --git a/src/models/channels/slack.ts b/src/models/channels/slack.ts index 40891a0..9d9425b 100644 --- a/src/models/channels/slack.ts +++ b/src/models/channels/slack.ts @@ -233,8 +233,7 @@ export class SlackChannelConfig extends ChannelConfig { data: JSON.stringify(body) }; try { - const result = await axios.request(config); - console.log(result); + await axios.request(config); } catch (err) { const msg = `Error posting to Slack: ${ err.message }`; log(msg, err);