From fa2d52a3b6a53739920364a345d3882af73ae3a8 Mon Sep 17 00:00:00 2001 From: Siddharth VP Date: Mon, 22 Apr 2024 21:39:12 +0530 Subject: [PATCH] bot-monitor: allow pausing email notifications --- .github/workflows/toolforge-deploy.yml | 2 +- bot-monitor/Alert.ts | 20 ++++++-- bot-monitor/AlertsDb.ts | 71 ++++++++++++++++++++------ bot-monitor/botmonitor.sql | 8 +++ bot-monitor/web-endpoint.hbs | 18 +++++++ bot-monitor/web-endpoint.ts | 61 ++++++++++++++++++++++ webservice/app.ts | 6 ++- webservice/package.json | 2 +- webservice/route-registry.ts | 2 + 9 files changed, 167 insertions(+), 23 deletions(-) create mode 100644 bot-monitor/botmonitor.sql create mode 100644 bot-monitor/web-endpoint.hbs create mode 100644 bot-monitor/web-endpoint.ts diff --git a/.github/workflows/toolforge-deploy.yml b/.github/workflows/toolforge-deploy.yml index 895bfb7..3636377 100644 --- a/.github/workflows/toolforge-deploy.yml +++ b/.github/workflows/toolforge-deploy.yml @@ -67,7 +67,7 @@ jobs: exit 1; fi; npm restart; - elif [[ "$(git diff --name-only HEAD HEAD@{1} | grep -c "web-endpoint")" -gt 0 || "$(git rev-list --format=%B --max-count=1 HEAD)" == *"!restart"* || "$(git rev-list --format=%B --max-count=1 HEAD)" == *"!web-restart"* ]]; then + elif [[ "$(git diff --name-only HEAD HEAD@{1} | grep -c "web-endpoint")" -gt 0 || "$(git diff --name-only HEAD HEAD@{1} | grep -c "hbs")" -gt 0 || "$(git rev-list --format=%B --max-count=1 HEAD)" == *"!restart"* || "$(git rev-list --format=%B --max-count=1 HEAD)" == *"!web-restart"* ]]; then echo "Restarting SDZeroBot webservice ..."; cd /data/project/sdzerobot/www/js && npm restart; fi; diff --git a/bot-monitor/Alert.ts b/bot-monitor/Alert.ts index 2c6e802..6e10e64 100644 --- a/bot-monitor/Alert.ts +++ b/bot-monitor/Alert.ts @@ -1,5 +1,6 @@ import { argv, bot, log, mailTransporter, Mwn } from "../botbase"; import {Rule, RuleError, Monitor, subtractFromNow, alertsDb} from "./index"; +import * as crypto from "crypto"; export class Alert { rule: Rule @@ -60,17 +61,18 @@ export class Alert { log(`[i] Sending email for "${this.name}" to ${this.rule.email}`); let subject = `[${this.rule.bot}] ${this.rule.task} failure`; + const webKey = crypto.randomBytes(32).toString('hex'); if (this.rule.email.includes('@')) { await mailTransporter.sendMail({ from: 'tools.sdzerobot@tools.wmflabs.org', to: this.rule.email, subject: subject, - html: this.getEmailBodyHtml(), + html: this.getEmailBodyHtml(webKey), }); } else { await new bot.User(this.rule.email).email( subject, - this.getEmailBodyPlain(), + this.getEmailBodyPlain(webKey), {ccme: true} ).catch(err => { if (err.code === 'notarget') { @@ -86,18 +88,26 @@ export class Alert { }); } - getEmailBodyHtml(): string { + getEmailBodyHtml(webKey: string): string { + const taskKey = `${this.rule.bot}: ${this.rule.task}`; return `${this.rule.bot}'s task ${this.rule.task} failed to run per the configuration specified at Wikipedia:Bot activity monitor/Configurations. Detected only ${this.actions} ${this.rule.action === 'edit' ? 'edit' : `"${this.rule.action}" action`}s in the last ${this.rule.duration}, whereas at least ${this.rule.minEdits} were expected.` + `

` + `If your bot is behaving as expected, then you may want to modify the task configuration instead. Or to unsubscribe from these email notifications, remove the |email= parameter from the {{/task}} template.` + `

` + + `To temporarily pause these notifications, click here: https://sdzerobot.toolforge.org/bot-monitor/pause?task=${encodeURIComponent(taskKey)}&webKey=${webKey}` + + `

` + `Thanks!`; } - getEmailBodyPlain(): string { + getEmailBodyPlain(webKey: string): string { + const taskKey = `${this.rule.bot}: ${this.rule.task}`; return `${this.rule.bot}'s task "${this.rule.task}" failed to run per the configuration specified at Wikipedia:Bot activity monitor/Configurations (). Detected only ${this.actions} ${this.rule.action === 'edit' ? 'edit' : `"${this.rule.action}" action`}s in the last ${this.rule.duration}, whereas at least ${this.rule.minEdits} were expected.` + `\n\n` + - `If your bot is behaving as expected, then you may want to modify the task configuration instead. Or to unsubscribe from these email notifications, remove the |email= parameter from the {{/task}} template. Thanks!`; + `If your bot is behaving as expected, then you may want to modify the task configuration instead. Or to unsubscribe from these email notifications, remove the |email= parameter from the {{/task}} template.` + + `\n\n` + + `To temporarily pause these notifications, click here: https://sdzerobot.toolforge.org/bot-monitor/pause?task=${encodeURIComponent(taskKey)}&webKey=${webKey}` + + `\n\n` + + `Thanks!`; } // static pingpage = 'Wikipedia:Bot activity monitor/Pings' diff --git a/bot-monitor/AlertsDb.ts b/bot-monitor/AlertsDb.ts index 36363eb..ea0abd8 100644 --- a/bot-monitor/AlertsDb.ts +++ b/bot-monitor/AlertsDb.ts @@ -3,43 +3,78 @@ import {MwnDate} from "mwn"; import {getKey, Rule} from "./Rule"; import {bot, toolsdb} from "../botbase"; import * as fs from "fs/promises"; +import * as crypto from "crypto"; import {getRedisInstance, Redis} from "../redis"; -import {createLocalSSHTunnel} from "../utils"; -import {TOOLS_DB_HOST} from "../db"; +import {ResultSetHeader} from "mysql2"; +import {CustomError} from "./web-endpoint"; interface AlertsDb { connect(): Promise; getLastEmailedTime(rule: Rule): Promise; saveLastEmailedTime(rule: Rule): Promise; + getPausedTillTime(bot: string, webKey: string): Promise; + setPausedTillTime(bot: string, webKey: string, pauseTill?: Date): Promise; } +// To allow user to disable checks for some time period: +// Generate a secret for each email sent and persist in db +// Provide a disable link with secret as query string in the email +// When clicked, check if secret is valid and disable notifications. + class MariadbAlertsDb implements AlertsDb { db: toolsdb; async connect(): Promise { - await createLocalSSHTunnel(TOOLS_DB_HOST); this.db = new toolsdb('botmonitor'); - await this.db.run(` - CREATE TABLE IF NOT EXISTS alerts( - name VARCHAR(255) PRIMARY KEY, - lastEmailed TIMESTAMP - ) - `); } async getLastEmailedTime(rule: Rule): Promise { - let data = await this.db.query( - `SELECT lastEmailed FROM alerts WHERE name = ?`, - [ getKey(rule, 250) ] + let data = await this.db.query(` + SELECT lastEmailed, paused FROM alerts + WHERE name = ? + `, [ getKey(rule, 250) ] ); - return data[0] ? new bot.Date(data[0].lastEmailed) : new bot.Date(0); + if (data[0]) { + if (data[0].paused && new bot.Date(data[0].paused).isAfter(new Date())) { + return new bot.Date(data[0].paused); + } + return new bot.Date(data[0].lastEmailed); + } else { + return new bot.Date(0); + } } async saveLastEmailedTime(rule: Rule): Promise { await this.db.run( - `REPLACE INTO alerts (name, lastEmailed) VALUES(?, UTC_TIMESTAMP())`, - [ getKey(rule, 250) ] + `REPLACE INTO alerts (name, lastEmailed, webKey) VALUES(?, UTC_TIMESTAMP(), ?)`, + [ getKey(rule, 250), crypto.randomBytes(32).toString('hex') ] ); } + + async getPausedTillTime(name: string, webKey: string) { + let data = await this.db.query(` + SELECT webKey, paused FROM alerts + WHERE name = ? + `, [name]); + if (!data[0]) { + throw new CustomError(404, 'No such bot task is configured.'); + } + if (data[0].webKey !== webKey) { + throw new CustomError(403, `Invalid or expired webKey. Please use the link from the latest SDZeroBot email.`); + } + if (data[0].paused) { + return new bot.Date(data[0].paused); + } + } + + async setPausedTillTime(name: string, webKey: string, pauseTill?: MwnDate): Promise { + const result = await this.db.run(` + UPDATE alerts + SET paused = ? + WHERE name = ? + AND webKey = ? + `, [pauseTill ? pauseTill.format('YYYY-MM-DD') : null, name, webKey]); + return (result?.[0] as ResultSetHeader)?.affectedRows; + } } class CassandraAlertsDb implements AlertsDb { @@ -63,6 +98,8 @@ class CassandraAlertsDb implements AlertsDb { [ getKey(rule) ] ); } + async getPausedTillTime(name: string, webKey: string) { return new bot.Date(0); } + async setPausedTillTime(bot: string, webKey: string, pauseTill: MwnDate) { return -1; } } class FileSystemAlertsDb implements AlertsDb { @@ -85,6 +122,8 @@ class FileSystemAlertsDb implements AlertsDb { // only needed on the last save, but done everytime anyway await fs.writeFile(this.file, JSON.stringify(this.data)); } + async getPausedTillTime(name: string, webKey: string) { return new bot.Date(0); } + async setPausedTillTime(bot: string, webKey: string, pauseTill: MwnDate) { return -1; } } class RedisAlertsDb implements AlertsDb { @@ -99,6 +138,8 @@ class RedisAlertsDb implements AlertsDb { async saveLastEmailedTime(rule: Rule) { await this.redis.hset('botmonitor-last-emailed', getKey(rule), new bot.date().toISOString); } + async getPausedTillTime(name: string, webKey: string) { return new bot.Date(0); } + async setPausedTillTime(bot: string, webKey: string, pauseTill: MwnDate) { return -1; } } export const alertsDb: AlertsDb = new MariadbAlertsDb(); diff --git a/bot-monitor/botmonitor.sql b/bot-monitor/botmonitor.sql new file mode 100644 index 0000000..404c0a5 --- /dev/null +++ b/bot-monitor/botmonitor.sql @@ -0,0 +1,8 @@ + +CREATE TABLE alerts( + name VARCHAR(255) PRIMARY KEY, + lastEmailed TIMESTAMP +); + +ALTER TABLE alerts ADD webKey CHAR(128); +ALTER TABLE alerts ADD paused timestamp null; diff --git a/bot-monitor/web-endpoint.hbs b/bot-monitor/web-endpoint.hbs new file mode 100644 index 0000000..aa27ba5 --- /dev/null +++ b/bot-monitor/web-endpoint.hbs @@ -0,0 +1,18 @@ +

Pausing failure notifications for {{task}}

+{{#if current}} +
+ + + + + +
+{{/if}} +
+
+ + + + + +
diff --git a/bot-monitor/web-endpoint.ts b/bot-monitor/web-endpoint.ts new file mode 100644 index 0000000..eaae109 --- /dev/null +++ b/bot-monitor/web-endpoint.ts @@ -0,0 +1,61 @@ +import * as express from "express"; +import {alertsDb} from "./AlertsDb"; +import {bot} from "../botbase"; + +const router = express.Router(); + +alertsDb.connect(); + +router.all('/pause', async (req, res, next) => { + try { + const { task, webKey } = req.query as Record; + const { date, unpause } = req.body as Record; + if (!webKey || !task) { + return next(new CustomError(400, "Missing one of required query params: task, webKey")); + } + + let current = ''; + let dateVal = ''; + if (date) { // POST + const tillDate = new bot.Date(date); + const rowsUpdated = await alertsDb.setPausedTillTime(task, webKey, tillDate); + if (!rowsUpdated) { + return next(new CustomError(403, "Unauthorized")); + } + current = `Successfully paused notifications till ${tillDate.format('D MMMM YYYY')} (UTC).`; + dateVal = tillDate.format('YYYY-MM-DD'); + } else if (unpause) { // POST + const rowsUpdated = await alertsDb.setPausedTillTime(task, webKey); + if (!rowsUpdated) { + return next(new CustomError(403, "Unauthorized")); + } + current = `Successfully unpaused notifications.`; + } else { // GET + let pausedTill = await alertsDb.getPausedTillTime(task, webKey); + if (pausedTill) { + current = `Notifications are currently paused till ${pausedTill.format('D MMMM YYYY')} (UTC).`; + dateVal = pausedTill.format('YYYY-MM-DD'); + } + } + + return res.render('bot-monitor/web-endpoint', { + task, + webKey, + current, + dateVal + }); + } catch (e) { + next(e); + } +}); + +export class CustomError extends Error { + status: number; + constructor(status: number, msg: string) { + super(msg); + this.status = status; + } +} + + +export default router; diff --git a/webservice/app.ts b/webservice/app.ts index 430e7db..885aed7 100644 --- a/webservice/app.ts +++ b/webservice/app.ts @@ -8,7 +8,7 @@ import * as cors from 'cors'; // All paths to SDZeroBot files must be via ../../SDZeroBot rather than via ../ // The latter will work locally but not when inside toolforge www/js directory! -import { bot, Mwn } from "../../SDZeroBot/botbase"; +import {bot, logFullError, Mwn} from "../../SDZeroBot/botbase"; import { humanDate } from "../../mwn/build/log"; import { registerRoutes } from "./route-registry"; @@ -64,6 +64,10 @@ app.use(function (err, req, res, next) { // render the error page res.status(err.status || 500); res.render('webservice/views/error'); + + if (!err.status || err.status === 500) { + logFullError(err, false); + } }); export default app; diff --git a/webservice/package.json b/webservice/package.json index 7307370..1dcdac5 100644 --- a/webservice/package.json +++ b/webservice/package.json @@ -4,7 +4,7 @@ "scripts": { "start": "env WEB=true /data/project/sdzerobot/bin/node server.js", "tunnels": "node tunnels.js", - "test": "nodemon server.ts", + "test": "nodemon server.ts --watch .. --ext ts,js,hbs", "debug": "node --require ts-node/register server.js", "restart": "webservice --backend kubernetes node18 restart", "logs": "kubectl logs deployment/sdzerobot" diff --git a/webservice/route-registry.ts b/webservice/route-registry.ts index 3e73f78..da25c73 100644 --- a/webservice/route-registry.ts +++ b/webservice/route-registry.ts @@ -8,6 +8,7 @@ import gansRouter from '../../SDZeroBot/most-gans/web-endpoint'; import articleSearchRouter from './routes/articlesearch'; import dykRouter from '../../SDZeroBot/dyk-counts/web-endpoint'; import gitsync from "./routes/gitsync"; +import botMonitorRouter from '../../SDZeroBot/bot-monitor/web-endpoint' export function registerRoutes(app: express.Router) { app.use('/', indexRouter); @@ -19,4 +20,5 @@ export function registerRoutes(app: express.Router) { app.use('/articlesearch', articleSearchRouter); app.use('/dyk', dykRouter); app.use('/gitsync', gitsync); + app.use('/bot-monitor', botMonitorRouter); }