From 4e5e2b20632c7ab366c49deda9ca388b9227c150 Mon Sep 17 00:00:00 2001 From: Hari Nugraha <15191978+haricnugraha@users.noreply.github.com> Date: Thu, 28 Mar 2024 12:15:11 +0700 Subject: [PATCH] Refactor: Convert TODO Comments Into Issues (#1262) * refactor: make sender configurable * fix: throw an error if operating system is unkown * refactor: move todo to https://github.com/hyperjumptech/monika/issues/1260 * refactor: todo has been covered by https://github.com/hyperjumptech/monika/issues/1229 * refactor: separate function due to bad abstraction * docs: fix typo and error example * test: remove failed test --- docs/src/pages/guides/tls-checkers.md | 3 +- docs/src/pages/quick-start.md | 2 +- packages/notification/channel/mailgun.ts | 6 +- packages/notification/channel/smtp.ts | 4 +- packages/notification/index.ts | 14 +- packages/notification/package.json | 2 +- packages/notification/utils/default-sender.ts | 19 +++ src/components/config/index.ts | 1 - src/components/logger/flush.test.ts | 31 ----- src/components/logger/history.ts | 2 - src/components/probe/prober/http/index.ts | 1 - src/jobs/tls-check.ts | 130 +++++++++++------- src/utils/open-website.ts | 3 +- src/utils/pino.ts | 1 - 14 files changed, 120 insertions(+), 99 deletions(-) create mode 100644 packages/notification/utils/default-sender.ts diff --git a/docs/src/pages/guides/tls-checkers.md b/docs/src/pages/guides/tls-checkers.md index 9be7705c8..967e59597 100644 --- a/docs/src/pages/guides/tls-checkers.md +++ b/docs/src/pages/guides/tls-checkers.md @@ -7,7 +7,8 @@ You can check TLS validity and set the threshold to send notification before the ```yaml probes: - - requests: + - id: 'Example' + requests: - url: http://example.com certificate: domains: diff --git a/docs/src/pages/quick-start.md b/docs/src/pages/quick-start.md index d8d7658df..e263b68f7 100644 --- a/docs/src/pages/quick-start.md +++ b/docs/src/pages/quick-start.md @@ -9,7 +9,7 @@ There are many ways to install Monika. However, currently only x64 architecture ### Windows -The recommendeded approach is to use [Chocolatey](https://community.chocolatey.org/packages/monika), a popular package manager for Windows. Check [Monika page on Chocolatey](https://community.chocolatey.org/packages/monika) for more detailed information. +The recommended approach is to use [Chocolatey](https://community.chocolatey.org/packages/monika), a popular package manager for Windows. Check [Monika page on Chocolatey](https://community.chocolatey.org/packages/monika) for more detailed information. If you have installed Chocolatey in your PC, then run the following command to install Monika: diff --git a/packages/notification/channel/mailgun.ts b/packages/notification/channel/mailgun.ts index afaf1301c..d34546829 100644 --- a/packages/notification/channel/mailgun.ts +++ b/packages/notification/channel/mailgun.ts @@ -26,6 +26,8 @@ import FormData from 'form-data' import Joi from 'joi' import type { NotificationMessage } from '.' import Mailgen from 'mailgen' + +import { getSender } from '../utils/default-sender' import { sendHttpRequest } from '../utils/http' export type NotificationData = { @@ -109,10 +111,8 @@ function getContent({ subject: string domain: string }): Content { - // TODO: Read from ENV Variables - const DEFAULT_SENDER_NAME = 'Monika' + const DEFAULT_SENDER_NAME = getSender().name const from = `${DEFAULT_SENDER_NAME} ` - const mailGenerator = new Mailgen({ theme: 'default', product: { diff --git a/packages/notification/channel/smtp.ts b/packages/notification/channel/smtp.ts index 43ddf4223..6dbc07e01 100644 --- a/packages/notification/channel/smtp.ts +++ b/packages/notification/channel/smtp.ts @@ -26,6 +26,7 @@ import * as nodemailer from 'nodemailer' import Mailgen from 'mailgen' import Joi from 'joi' +import { getSender } from '../utils/default-sender' import type { NotificationMessage } from '.' type NotificationData = { @@ -51,8 +52,7 @@ export const send = async ( { hostname, password, port, recipients, username }: NotificationData, { body, subject }: NotificationMessage ): Promise => { - // TODO: Read from ENV Variables - const DEFAULT_EMAIL = 'monika@hyperjump.tech' + const DEFAULT_EMAIL = getSender().email const DEFAULT_SENDER_NAME = 'Monika' const transporter = nodemailer.createTransport({ host: hostname, diff --git a/packages/notification/index.ts b/packages/notification/index.ts index a25d83301..2876c0a9f 100644 --- a/packages/notification/index.ts +++ b/packages/notification/index.ts @@ -22,13 +22,23 @@ * SOFTWARE. * **********************************************************************************/ -import { channels, Notification, NotificationMessage } from './channel' +import { + channels, + type Notification, + type NotificationMessage, +} from './channel' import { getErrorMessage } from './utils/catch-error-handler' +import { type InputSender, updateSender } from './utils/default-sender' async function sendNotifications( notifications: Notification[], - message: NotificationMessage + message: NotificationMessage, + sender?: InputSender ): Promise { + if (sender) { + updateSender(sender) + } + await Promise.all( notifications.map(async ({ data, type }) => { const channel = channels[type] diff --git a/packages/notification/package.json b/packages/notification/package.json index c839713fe..bf96f27e9 100644 --- a/packages/notification/package.json +++ b/packages/notification/package.json @@ -1,6 +1,6 @@ { "name": "@hyperjumptech/monika-notification", - "version": "1.16.1", + "version": "1.17.0", "description": "notification package for monika", "main": "lib/index.js", "types": "lib/index.d.ts", diff --git a/packages/notification/utils/default-sender.ts b/packages/notification/utils/default-sender.ts new file mode 100644 index 000000000..962438ca4 --- /dev/null +++ b/packages/notification/utils/default-sender.ts @@ -0,0 +1,19 @@ +type Sender = { + name: string + email: string +} + +export type InputSender = Partial + +let defaultSender: Sender = { + name: 'Monika', + email: 'monika@hyperjump.tech', +} + +export function getSender(): Sender { + return defaultSender +} + +export function updateSender(sender: InputSender) { + defaultSender = { ...defaultSender, ...sender } +} diff --git a/src/components/config/index.ts b/src/components/config/index.ts index 928eb4e7b..27e95e0a3 100644 --- a/src/components/config/index.ts +++ b/src/components/config/index.ts @@ -232,7 +232,6 @@ function watchConfigFile({ flags, path }: WatchConfigFileParams) { } const getPathAndTypeFromFlag = (flags: MonikaFlags) => { - // TODO: Assuming the first index of config is the primary config let path = flags.config?.[0] let type = 'monika' diff --git a/src/components/logger/flush.test.ts b/src/components/logger/flush.test.ts index 38423376b..b9db1864d 100644 --- a/src/components/logger/flush.test.ts +++ b/src/components/logger/flush.test.ts @@ -23,10 +23,7 @@ **********************************************************************************/ import sinon from 'sinon' -import { ux } from '@oclif/core' -import { test } from '@oclif/test' import * as history from './history' -import cmd from '../../commands/monika' import { flush } from './flush' let flushAllLogsStub: sinon.SinonStub @@ -49,32 +46,4 @@ describe('Flush command', () => { sinon.assert.calledOnce(flushAllLogsStub) }) }) - - describe('Not force', () => { - test - // TODO: Remove skip - .skip() - // arrange - .stub(ux.ux, 'prompt', (stub) => stub.resolves('n')) - .stdout() - // act - .do(() => cmd.run(['--flush'])) - .it('should cancel flush', () => { - // assert - sinon.assert.notCalled(flushAllLogsStub) - }) - - test - // TODO: Remove skip - .skip() - // arrange - .stub(ux.ux, 'prompt', (stub) => stub.resolves('Y')) - .stdout() - // act - .do(() => cmd.run(['--flush'])) - .it('should flush', () => { - // assert - sinon.assert.calledOnce(flushAllLogsStub) - }) - }) }) diff --git a/src/components/logger/history.ts b/src/components/logger/history.ts index c2e625ba5..cfe9eee45 100644 --- a/src/components/logger/history.ts +++ b/src/components/logger/history.ts @@ -135,7 +135,6 @@ export function setDatabase( async function migrate() { await database().migrate({ - // TODO: Current vercel/pkg is dependent with CommonJS // eslint-disable-next-line unicorn/prefer-module migrationsPath: path.join(__dirname, '../../../db/migrations'), }) @@ -448,7 +447,6 @@ export async function saveProbeRequestLog({ const now = Math.round(Date.now() / 1000) const requestConfig = probe.requests?.[requestIndex] - // TODO: limit data stored. const responseBody = requestConfig?.saveBody ? typeof probeRes.data === 'string' ? probeRes.data diff --git a/src/components/probe/prober/http/index.ts b/src/components/probe/prober/http/index.ts index d488d34a6..0d0541208 100644 --- a/src/components/probe/prober/http/index.ts +++ b/src/components/probe/prober/http/index.ts @@ -260,7 +260,6 @@ function getProbeResultMessage({ request, response, }: ProbeResultMessageParams): string { - // TODO: make this more generic not probe dependent if (request?.ping) { return response?.body as string } diff --git a/src/jobs/tls-check.ts b/src/jobs/tls-check.ts index 34bddf441..00b2f57a1 100644 --- a/src/jobs/tls-check.ts +++ b/src/jobs/tls-check.ts @@ -22,20 +22,24 @@ * SOFTWARE. * **********************************************************************************/ +import { + sendNotifications, + type Notification, + type NotificationMessage, +} from '@hyperjumptech/monika-notification' +import { format } from 'date-fns' + import { getConfig } from '../components/config' import { saveNotificationLog } from '../components/logger/history' -import { sendAlerts } from '../components/notification' +import { + getMonikaInstance, + getOSName, +} from '../components/notification/alert-message' import { checkTLS, getHostname } from '../components/tls-checker' -import type { Notification } from '@hyperjumptech/monika-notification' -import type { ValidatedResponse } from '../plugins/validate-response' +import { getContext } from '../context' +import getIp from '../utils/ip' import { log } from '../utils/pino' -import { probeRequestResult } from '../interfaces/request' - -type SendTLSErrorNotificationProps = { - hostname: string - notifications: Notification[] - errorMessage: string -} +import { publicIpAddress } from '../utils/public-ip' export function tlsChecker(): void { const config = getConfig() @@ -64,62 +68,86 @@ export function tlsChecker(): void { return } - sendTLSErrorNotification({ + sendErrorNotification({ + errorMessage: error.message, hostname, notifications: notifications || [], - errorMessage: error.message, - }) + }).catch(console.error) }) } } -function sendTLSErrorNotification({ +type SendErrorNotificationParams = { + errorMessage: string + hostname: string + notifications: Notification[] +} + +async function sendErrorNotification({ + errorMessage, hostname, notifications, - errorMessage, -}: SendTLSErrorNotificationProps) { - // TODO: Remove probe below - // probe is used because probe detail is needed to save the notification log +}: SendErrorNotificationParams) { const probe = { id: '', + alerts: [], + interval: 10, name: '', requests: [], - interval: 10, - alerts: [], } - for (const notification of notifications) { - // TODO: Remove validation below - // validation is used because it is needed to send alert - const validation: ValidatedResponse = { - alert: { - id: '', - assertion: '', - message: errorMessage, - }, - isAlertTriggered: true, - response: { - status: 500, - responseTime: 0, - data: {}, - body: {}, - headers: {}, - result: probeRequestResult.success, - }, - } - - saveNotificationLog(probe, notification, 'NOTIFY-TLS', errorMessage).catch( - (error) => log.error(error.message) - ) - - // TODO: invoke sendNotifications function instead - // looks like the sendAlerts function does not handle this - sendAlerts({ + await Promise.allSettled( + notifications.map(async (notification) => { + await sendNotifications( + notifications, + await getNotificationMessage({ hostname, errorMessage }) + ) + await saveNotificationLog(probe, notification, 'NOTIFY-TLS', errorMessage) + }) + ) +} + +type GetMessageParams = { + hostname: string + errorMessage: string +} + +async function getNotificationMessage({ + hostname, + errorMessage, +}: GetMessageParams): Promise { + const privateIpAddress = getIp() + const { userAgent } = getContext() + const [monikaInstance, osName] = await Promise.all([ + getMonikaInstance(privateIpAddress), + getOSName(), + ]) + const time = format(new Date(), 'yyyy-MM-dd HH:mm:ss XXX') + const body = `Message: ${errorMessage} + + URL: ${hostname} + + Time: ${time} + + From: ${monikaInstance} + + OS: ${osName} + + Version: ${userAgent}` + + return { + subject: 'New Incident from Monika', + body, + summary: errorMessage, + meta: { + hostname, + privateIpAddress, probeID: '', + publicIpAddress, + time, + type: 'incident', url: hostname, - probeState: 'invalid', - notifications: notifications ?? [], - validation, - }).catch((error) => log.error(error.message)) + version: userAgent, + }, } } diff --git a/src/utils/open-website.ts b/src/utils/open-website.ts index 5bf5e7c1f..7d6c7610a 100644 --- a/src/utils/open-website.ts +++ b/src/utils/open-website.ts @@ -44,8 +44,7 @@ export const open = (url: string): void => { } default: { - // TODO: Handle new OS - break + throw new Error(`Unknown operating system: ${operatingSystem}`) } } } diff --git a/src/utils/pino.ts b/src/utils/pino.ts index c536b353e..1825d40a4 100644 --- a/src/utils/pino.ts +++ b/src/utils/pino.ts @@ -43,7 +43,6 @@ function getOptions() { hideObject: true, } - // TODO: Current vercel/pkg is dependent with CommonJS // eslint-disable-next-line unicorn/prefer-module const project = path.join(__dirname, '../../tsconfig.json') const dev = fs.existsSync(project)