feat: Enhance Microsoft Rewards Bot with recovery email normalization and improved logging

- Added `normalizeRecoveryEmail` utility function for consistent recovery email validation.
- Improved logging functionality in `Logger.ts` with enhanced edge case handling and null checks.
- Centralized browser cleanup logic in `BrowserFactory.ts` to eliminate duplication.
- Refactored error handling and message formatting in `Utils.ts` for better clarity and consistency.
- Updated various log messages for improved readability and consistency across the codebase.
- Implemented periodic cleanup of webhook buffers in `Logger.ts` using centralized constants.
This commit is contained in:
2025-11-11 12:31:27 +01:00
parent 6b7b07cc1f
commit 088a3a024f
7 changed files with 277 additions and 199 deletions

View File

@@ -116,3 +116,8 @@ export const DISCORD = {
WEBHOOK_USERNAME: 'Microsoft-Rewards-Bot', WEBHOOK_USERNAME: 'Microsoft-Rewards-Bot',
AVATAR_URL: 'https://raw.githubusercontent.com/Obsidian-wtf/Microsoft-Rewards-Bot/main/assets/logo.png' AVATAR_URL: 'https://raw.githubusercontent.com/Obsidian-wtf/Microsoft-Rewards-Bot/main/assets/logo.png'
} as const } as const
export const LOGGER_CLEANUP = {
BUFFER_MAX_AGE_MS: TIMEOUTS.ONE_HOUR,
BUFFER_CLEANUP_INTERVAL_MS: TIMEOUTS.TEN_MINUTES
} as const

View File

@@ -12,7 +12,7 @@
import type { MicrosoftRewardsBot } from '../index' import type { MicrosoftRewardsBot } from '../index'
import type { Account } from '../interface/Account' import type { Account } from '../interface/Account'
import { createBrowserInstance } from '../util/BrowserFactory' import { closeBrowserSafely, createBrowserInstance } from '../util/BrowserFactory'
import { handleCompromisedMode } from './FlowUtils' import { handleCompromisedMode } from './FlowUtils'
export interface DesktopFlowResult { export interface DesktopFlowResult {
@@ -136,12 +136,8 @@ export class DesktopFlow {
} }
} finally { } finally {
if (!keepBrowserOpen) { if (!keepBrowserOpen) {
try { // IMPROVED: Use centralized browser close utility to eliminate duplication
await this.bot.browser.func.closeBrowser(browser, account.email) await closeBrowserSafely(this.bot, browser, account.email, false)
} catch (closeError) {
const message = closeError instanceof Error ? closeError.message : String(closeError)
this.bot.log(false, 'DESKTOP-FLOW', `Failed to close desktop context: ${message}`, 'warn')
}
} }
} }
} }

View File

@@ -13,7 +13,7 @@
import type { MicrosoftRewardsBot } from '../index' import type { MicrosoftRewardsBot } from '../index'
import type { Account } from '../interface/Account' import type { Account } from '../interface/Account'
import { createBrowserInstance } from '../util/BrowserFactory' import { closeBrowserSafely, createBrowserInstance } from '../util/BrowserFactory'
import { MobileRetryTracker } from '../util/MobileRetryTracker' import { MobileRetryTracker } from '../util/MobileRetryTracker'
import { handleCompromisedMode } from './FlowUtils' import { handleCompromisedMode } from './FlowUtils'
@@ -83,7 +83,13 @@ export class MobileFlow {
await this.bot.browser.func.goHome(this.bot.homePage) await this.bot.browser.func.goHome(this.bot.homePage)
const data = await this.bot.browser.func.getDashboardData() const data = await this.bot.browser.func.getDashboardData()
const initialPoints = data.userStatus.availablePoints || 0
// FIXED: Log warning when availablePoints is missing instead of silently defaulting
const initialPoints = data.userStatus.availablePoints
if (initialPoints === undefined || initialPoints === null) {
this.bot.log(true, 'MOBILE-FLOW', 'Warning: availablePoints is undefined/null, defaulting to 0. This may indicate dashboard data issues.', 'warn')
}
const safeInitialPoints = initialPoints ?? 0
const browserEarnablePoints = await this.bot.browser.func.getBrowserEarnablePoints() const browserEarnablePoints = await this.bot.browser.func.getBrowserEarnablePoints()
const appEarnablePoints = await this.bot.browser.func.getAppEarnablePoints(accessToken) const appEarnablePoints = await this.bot.browser.func.getAppEarnablePoints(accessToken)
@@ -102,7 +108,7 @@ export class MobileFlow {
this.bot.log(true, 'MOBILE-FLOW', 'No points to earn and "runOnZeroPoints" is set to "false", stopping!', 'log', 'yellow') this.bot.log(true, 'MOBILE-FLOW', 'No points to earn and "runOnZeroPoints" is set to "false", stopping!', 'log', 'yellow')
return { return {
initialPoints: initialPoints, initialPoints: safeInitialPoints,
collectedPoints: 0 collectedPoints: 0
} }
} }
@@ -146,13 +152,9 @@ export class MobileFlow {
this.bot.log(true, 'MOBILE-FLOW', `Attempt ${attempt}/${maxMobileRetries}: Unable to complete mobile searches, bad User-Agent? Increase search delay? Retrying...`, 'log', 'yellow') this.bot.log(true, 'MOBILE-FLOW', `Attempt ${attempt}/${maxMobileRetries}: Unable to complete mobile searches, bad User-Agent? Increase search delay? Retrying...`, 'log', 'yellow')
// Close mobile browser before retrying to release resources // Close mobile browser before retrying to release resources
try { // IMPROVED: Use centralized browser close utility
await this.bot.browser.func.closeBrowser(browser, account.email) await closeBrowserSafely(this.bot, browser, account.email, true)
browserClosed = true browserClosed = true
} catch (closeError) {
const message = closeError instanceof Error ? closeError.message : String(closeError)
this.bot.log(true, 'MOBILE-FLOW', `Failed to close mobile context before retry: ${message}`, 'warn')
}
// Create a new browser and try again with the same tracker // Create a new browser and try again with the same tracker
return await this.run(account, retryTracker) return await this.run(account, retryTracker)
@@ -165,21 +167,17 @@ export class MobileFlow {
const afterPointAmount = await this.bot.browser.func.getCurrentPoints() const afterPointAmount = await this.bot.browser.func.getCurrentPoints()
this.bot.log(true, 'MOBILE-FLOW', `The script collected ${afterPointAmount - initialPoints} points today`) this.bot.log(true, 'MOBILE-FLOW', `The script collected ${afterPointAmount - safeInitialPoints} points today`)
return { return {
initialPoints: initialPoints, initialPoints: safeInitialPoints,
collectedPoints: (afterPointAmount - initialPoints) || 0 collectedPoints: (afterPointAmount - safeInitialPoints) || 0
} }
} finally { } finally {
if (!keepBrowserOpen && !browserClosed) { if (!keepBrowserOpen && !browserClosed) {
try { // IMPROVED: Use centralized browser close utility to eliminate duplication
await this.bot.browser.func.closeBrowser(browser, account.email) await closeBrowserSafely(this.bot, browser, account.email, true)
browserClosed = true browserClosed = true
} catch (closeError) {
const message = closeError instanceof Error ? closeError.message : String(closeError)
this.bot.log(true, 'MOBILE-FLOW', `Failed to close mobile context: ${message}`, 'warn')
}
} }
} }
} }

View File

@@ -16,7 +16,7 @@ import { log } from './util/Logger'
import { MobileRetryTracker } from './util/MobileRetryTracker' import { MobileRetryTracker } from './util/MobileRetryTracker'
import { QueryDiversityEngine } from './util/QueryDiversityEngine' import { QueryDiversityEngine } from './util/QueryDiversityEngine'
import { StartupValidator } from './util/StartupValidator' import { StartupValidator } from './util/StartupValidator'
import { formatDetailedError, shortErrorMessage, Util } from './util/Utils' import { formatDetailedError, normalizeRecoveryEmail, shortErrorMessage, Util } from './util/Utils'
import { Activities } from './functions/Activities' import { Activities } from './functions/Activities'
import { Login } from './functions/Login' import { Login } from './functions/Login'
@@ -427,9 +427,8 @@ export class MicrosoftRewardsBot {
} }
} catch {/* ignore */ } } catch {/* ignore */ }
this.currentAccountEmail = account.email this.currentAccountEmail = account.email
this.currentAccountRecoveryEmail = (typeof account.recoveryEmail === 'string' && account.recoveryEmail.trim() !== '') // IMPROVED: Use centralized recovery email validation utility
? account.recoveryEmail.trim() this.currentAccountRecoveryEmail = normalizeRecoveryEmail(account.recoveryEmail)
: undefined
const runNumber = (this.accountRunCounts.get(account.email) ?? 0) + 1 const runNumber = (this.accountRunCounts.get(account.email) ?? 0) + 1
this.accountRunCounts.set(account.email, runNumber) this.accountRunCounts.set(account.email, runNumber)
log('main', 'MAIN-WORKER', `Started tasks for account ${account.email}`) log('main', 'MAIN-WORKER', `Started tasks for account ${account.email}`)

View File

@@ -2,7 +2,7 @@
* Browser Factory Utility * Browser Factory Utility
* Eliminates code duplication between Desktop and Mobile flows * Eliminates code duplication between Desktop and Mobile flows
* *
* Centralized browser instance creation logic * Centralized browser instance creation and cleanup logic
*/ */
import type { BrowserContext } from 'rebrowser-playwright' import type { BrowserContext } from 'rebrowser-playwright'
@@ -31,3 +31,30 @@ export async function createBrowserInstance(
const browserInstance = new Browser(bot) const browserInstance = new Browser(bot)
return await browserInstance.createBrowser(proxy, email) return await browserInstance.createBrowser(proxy, email)
} }
/**
* Safely close browser context with error handling
* IMPROVEMENT: Extracted from DesktopFlow and MobileFlow to eliminate duplication
*
* @param bot Bot instance
* @param browser Browser context to close
* @param email Account email for logging
* @param isMobile Whether this is a mobile browser context
*
* @example
* await closeBrowserSafely(bot, browser, account.email, false)
*/
export async function closeBrowserSafely(
bot: MicrosoftRewardsBot,
browser: BrowserContext,
email: string,
isMobile: boolean
): Promise<void> {
try {
await bot.browser.func.closeBrowser(browser, email)
} catch (closeError) {
const message = closeError instanceof Error ? closeError.message : String(closeError)
const platform = isMobile ? 'mobile' : 'desktop'
bot.log(isMobile, `${platform.toUpperCase()}-FLOW`, `Failed to close ${platform} context: ${message}`, 'warn')
}
}

View File

@@ -1,6 +1,6 @@
import axios from 'axios' import axios from 'axios'
import chalk from 'chalk' import chalk from 'chalk'
import { DISCORD, TIMEOUTS } from '../constants' import { DISCORD, LOGGER_CLEANUP } from '../constants'
import { sendErrorReport } from './ErrorReportingWebhook' import { sendErrorReport } from './ErrorReportingWebhook'
import { loadConfig } from './Load' import { loadConfig } from './Load'
import { Ntfy } from './Ntfy' import { Ntfy } from './Ntfy'
@@ -26,22 +26,19 @@ type WebhookBuffer = {
const webhookBuffers = new Map<string, WebhookBuffer>() const webhookBuffers = new Map<string, WebhookBuffer>()
// Periodic cleanup of old/idle webhook buffers to prevent memory leaks // Periodic cleanup of old/idle webhook buffers to prevent memory leaks
// IMPROVED: Using centralized constants instead of magic numbers // IMPROVED: Using centralized constants from constants.ts
const BUFFER_MAX_AGE_MS = TIMEOUTS.ONE_HOUR
const BUFFER_CLEANUP_INTERVAL_MS = TIMEOUTS.TEN_MINUTES
const cleanupInterval = setInterval(() => { const cleanupInterval = setInterval(() => {
const now = Date.now() const now = Date.now()
for (const [url, buf] of webhookBuffers.entries()) { for (const [url, buf] of webhookBuffers.entries()) {
if (!buf.sending && buf.lines.length === 0) { if (!buf.sending && buf.lines.length === 0) {
const lastActivity = (buf as unknown as { lastActivity?: number }).lastActivity || 0 const lastActivity = (buf as unknown as { lastActivity?: number }).lastActivity || 0
if (now - lastActivity > BUFFER_MAX_AGE_MS) { if (now - lastActivity > LOGGER_CLEANUP.BUFFER_MAX_AGE_MS) {
webhookBuffers.delete(url) webhookBuffers.delete(url)
} }
} }
} }
}, BUFFER_CLEANUP_INTERVAL_MS) }, LOGGER_CLEANUP.BUFFER_CLEANUP_INTERVAL_MS)
// FIXED: Allow cleanup to be stopped with proper fallback // FIXED: Allow cleanup to be stopped with proper fallback
// unref() prevents process from hanging but may not exist in all environments // unref() prevents process from hanging but may not exist in all environments
@@ -149,13 +146,44 @@ function determineColorFromContent(content: string): number {
/** /**
* Type guard to check if config has valid logging configuration * Type guard to check if config has valid logging configuration
* IMPROVED: Enhanced edge case handling and null checks
*/ */
function hasValidLogging(config: unknown): config is { logging: { excludeFunc?: string[]; webhookExcludeFunc?: string[] } } { function hasValidLogging(config: unknown): config is { logging: { excludeFunc?: string[]; webhookExcludeFunc?: string[]; redactEmails?: boolean; liveWebhookUrl?: string } } {
return typeof config === 'object' && if (typeof config !== 'object' || config === null) {
config !== null && return false
'logging' in config && }
typeof config.logging === 'object' &&
config.logging !== null if (!('logging' in config)) {
return false
}
const cfg = config as Record<string, unknown>
const logging = cfg.logging
if (typeof logging !== 'object' || logging === null) {
return false
}
// Validate optional fields have correct types if present
const loggingObj = logging as Record<string, unknown>
if ('excludeFunc' in loggingObj && !Array.isArray(loggingObj.excludeFunc)) {
return false
}
if ('webhookExcludeFunc' in loggingObj && !Array.isArray(loggingObj.webhookExcludeFunc)) {
return false
}
if ('redactEmails' in loggingObj && typeof loggingObj.redactEmails !== 'boolean') {
return false
}
if ('liveWebhookUrl' in loggingObj && typeof loggingObj.liveWebhookUrl !== 'string') {
return false
}
return true
} }
function enqueueWebhookLog(url: string, line: string) { function enqueueWebhookLog(url: string, line: string) {

View File

@@ -35,19 +35,23 @@ export class Util {
/** /**
* Wait for a specified number of milliseconds * Wait for a specified number of milliseconds
* @param ms - Milliseconds to wait (max 1 hour) * @param ms - Milliseconds to wait (max 1 hour, min 0)
* @throws {Error} If ms is not finite or is NaN/Infinity * @throws {Error} If ms is not finite, is NaN/Infinity, or is negative
* @example await utils.wait(1000) // Wait 1 second * @example await utils.wait(1000) // Wait 1 second
*/ */
wait(ms: number): Promise<void> { wait(ms: number): Promise<void> {
const MAX_WAIT_MS = 3600000 // 1 hour max to prevent infinite waits const MAX_WAIT_MS = 3600000 // 1 hour max to prevent infinite waits
const MIN_WAIT_MS = 0 const MIN_WAIT_MS = 0
// FIXED: Simplified validation - isFinite checks both NaN and Infinity // FIXED: Comprehensive validation - check finite, NaN, Infinity, and negative values
if (!Number.isFinite(ms)) { if (!Number.isFinite(ms)) {
throw new Error(`Invalid wait time: ${ms}. Must be a finite number (not NaN or Infinity).`) throw new Error(`Invalid wait time: ${ms}. Must be a finite number (not NaN or Infinity).`)
} }
if (ms < 0) {
throw new Error(`Invalid wait time: ${ms}. Cannot wait negative milliseconds.`)
}
const safeMs = Math.min(Math.max(MIN_WAIT_MS, ms), MAX_WAIT_MS) const safeMs = Math.min(Math.max(MIN_WAIT_MS, ms), MAX_WAIT_MS)
return new Promise<void>((resolve) => { return new Promise<void>((resolve) => {
@@ -215,3 +219,24 @@ export function formatDetailedError(label: string, error: unknown, includeStack:
} }
return `${label}:${baseMessage}` return `${label}:${baseMessage}`
} }
/**
* Validate and normalize recovery email
* IMPROVEMENT: Extracted to eliminate duplication and provide consistent validation
*
* @param recoveryEmail - Raw recovery email value from account configuration
* @returns Normalized recovery email string or undefined if invalid
*
* @example
* normalizeRecoveryEmail(' test@example.com ') // 'test@example.com'
* normalizeRecoveryEmail('') // undefined
* normalizeRecoveryEmail(undefined) // undefined
*/
export function normalizeRecoveryEmail(recoveryEmail: unknown): string | undefined {
if (typeof recoveryEmail !== 'string') {
return undefined
}
const trimmed = recoveryEmail.trim()
return trimmed === '' ? undefined : trimmed
}