feat: Centralize timeout constants and improve logging validation across modules

This commit is contained in:
2025-11-09 18:15:01 +01:00
parent 2c55fff61d
commit 3a3f416d92
4 changed files with 73 additions and 13 deletions

View File

@@ -12,6 +12,15 @@
* @param max Maximum allowed value
* @returns Parsed number or default value
*/
/**
* Parse environment variable as number with validation
* FIXED: Added strict validation for min/max boundaries with centralized logging
* @param key Environment variable name
* @param defaultValue Default value if parsing fails or out of range
* @param min Minimum allowed value
* @param max Maximum allowed value
* @returns Parsed number or default value
*/
function parseEnvNumber(key: string, defaultValue: number, min: number, max: number): number {
const raw = process.env[key]
if (!raw) return defaultValue
@@ -19,12 +28,27 @@ function parseEnvNumber(key: string, defaultValue: number, min: number, max: num
const parsed = Number(raw)
// Strict validation: must be finite, not NaN, and within bounds
if (!Number.isFinite(parsed)) {
console.warn(`[Constants] Invalid ${key}="${raw}" (not a finite number), using default: ${defaultValue}`)
// Defer logging import to avoid circular dependency during module initialization
// Log only happens on actual misconfiguration (rare edge case)
queueMicrotask(() => {
import('./util/Logger').then(({ log }) => {
log('main', 'CONSTANTS', `Invalid ${key}="${raw}" (not a finite number), using default: ${defaultValue}`, 'warn')
}).catch(() => {
// Fallback if logger unavailable during initialization
process.stderr.write(`[Constants] Invalid ${key}="${raw}" (not a finite number), using default: ${defaultValue}\n`)
})
})
return defaultValue
}
if (parsed < min || parsed > max) {
console.warn(`[Constants] ${key}=${parsed} out of range [${min}, ${max}], using default: ${defaultValue}`)
queueMicrotask(() => {
import('./util/Logger').then(({ log }) => {
log('main', 'CONSTANTS', `${key}=${parsed} out of range [${min}, ${max}], using default: ${defaultValue}`, 'warn')
}).catch(() => {
process.stderr.write(`[Constants] ${key}=${parsed} out of range [${min}, ${max}], using default: ${defaultValue}\n`)
})
})
return defaultValue
}
@@ -47,6 +71,8 @@ export const TIMEOUTS = {
LOGIN_MAX: parseEnvNumber('LOGIN_MAX_WAIT_MS', LOGIN_TIMEOUT_DEFAULT_MS, LOGIN_TIMEOUT_MIN_MS, LOGIN_TIMEOUT_MAX_MS),
NETWORK_IDLE: 5000,
ONE_MINUTE: 60000,
FIVE_MINUTES: 300000,
TEN_MINUTES: 600000,
ONE_HOUR: 3600000,
TWO_MINUTES: 120000
} as const

View File

@@ -3,6 +3,7 @@ import * as crypto from 'crypto'
import type { Locator, Page } from 'playwright'
import readline from 'readline'
import { TIMEOUTS } from '../constants'
import { MicrosoftRewardsBot } from '../index'
import { OAuth } from '../interface/OAuth'
import { saveSessionData } from '../util/Load'
@@ -399,6 +400,9 @@ export class Login {
this.bot.log(this.bot.isMobile, 'LOGIN-APP', `Token exchange failed (network error): ${errMsg}`, 'error')
}
throw error
} finally {
// Always cleanup compromised interval to prevent memory leaks
this.cleanupCompromisedInterval()
}
}
@@ -1696,6 +1700,7 @@ export class Login {
clearInterval(this.compromisedInterval)
this.compromisedInterval = undefined
}
// IMPROVED: Using centralized constant instead of magic number (5*60*1000)
this.compromisedInterval = setInterval(()=>{
try {
this.bot.log(this.bot.isMobile,'SECURITY','Security standby active. Manual review required before proceeding.','warn')
@@ -1703,7 +1708,7 @@ export class Login {
// Intentionally silent: If logging fails in interval, don't crash the timer
// The interval will try again in 5 minutes
}
}, 5*60*1000)
}, TIMEOUTS.FIVE_MINUTES)
}
private cleanupCompromisedInterval() {

View File

@@ -310,11 +310,9 @@ export class MicrosoftRewardsBot {
worker.send({ chunk })
}
worker.on('message', (msg: unknown) => {
// FIXED: Validate message structure before accessing properties
if (!msg || typeof msg !== 'object') return
const m = msg as { type?: string; data?: AccountSummary[] }
if (m && m.type === 'summary' && Array.isArray(m.data)) {
this.accountSummaries.push(...m.data)
// IMPROVED: Using type-safe interface and type guard
if (isWorkerMessage(msg)) {
this.accountSummaries.push(...msg.data)
}
})
}
@@ -345,8 +343,10 @@ export class MicrosoftRewardsBot {
}
newW.on('message', (msg: unknown) => {
const m = msg as { type?: string; data?: AccountSummary[] }
if (m && m.type === 'summary' && Array.isArray(m.data)) this.accountSummaries.push(...m.data)
// IMPROVED: Using type-safe interface and type guard
if (isWorkerMessage(msg)) {
this.accountSummaries.push(...msg.data)
}
})
}
}
@@ -485,12 +485,17 @@ export class MicrosoftRewardsBot {
if (this.config.parallel) {
const mobileInstance = new MicrosoftRewardsBot(true)
mobileInstance.axios = this.axios
// IMPROVED: Shared state to track desktop issues for early mobile abort consideration
let desktopDetectedIssue = false
// Run both and capture results with detailed logging
const desktopPromise = this.Desktop(account).catch((e: unknown) => {
const msg = e instanceof Error ? e.message : String(e)
log(false, 'TASK', `Desktop flow failed early for ${account.email}: ${msg}`,'error')
const bd = detectBanReason(e)
if (bd.status) {
desktopDetectedIssue = true // Track issue for logging
banned.status = true; banned.reason = bd.reason.substring(0,200)
void this.handleImmediateBanAlert(account.email, banned.reason)
}
@@ -508,6 +513,11 @@ export class MicrosoftRewardsBot {
})
const [desktopResult, mobileResult] = await Promise.allSettled([desktopPromise, mobilePromise])
// Log if desktop detected issue (helps identify when both flows ran despite ban)
if (desktopDetectedIssue) {
log('main', 'TASK', `Desktop detected security issue for ${account.email} during parallel execution. Future enhancement: implement AbortController for early mobile cancellation.`, 'warn')
}
// Handle desktop result
if (desktopResult.status === 'fulfilled' && desktopResult.value) {
desktopInitial = desktopResult.value.initialPoints
@@ -858,6 +868,24 @@ interface AccountSummary {
banned?: { status: boolean; reason: string }
}
/**
* IMPROVED: Type-safe worker message interface
* Replaces inline type assertion for better type safety
*/
interface WorkerMessage {
type: 'summary'
data: AccountSummary[]
}
/**
* Type guard to validate worker message structure
*/
function isWorkerMessage(msg: unknown): msg is WorkerMessage {
if (!msg || typeof msg !== 'object') return false
const m = msg as Partial<WorkerMessage>
return m.type === 'summary' && Array.isArray(m.data)
}
function shortErr(e: unknown): string {
if (e == null) return 'unknown'
if (e instanceof Error) return e.message.substring(0, 120)

View File

@@ -1,7 +1,7 @@
import axios from 'axios'
import chalk from 'chalk'
import { DISCORD } from '../constants'
import { DISCORD, TIMEOUTS } from '../constants'
import { sendErrorReport } from './ErrorReportingWebhook'
import { loadConfig } from './Load'
import { Ntfy } from './Ntfy'
@@ -27,8 +27,9 @@ type WebhookBuffer = {
const webhookBuffers = new Map<string, WebhookBuffer>()
// Periodic cleanup of old/idle webhook buffers to prevent memory leaks
const BUFFER_MAX_AGE_MS = 3600000 // 1 hour
const BUFFER_CLEANUP_INTERVAL_MS = 600000 // 10 minutes
// IMPROVED: Using centralized constants instead of magic numbers
const BUFFER_MAX_AGE_MS = TIMEOUTS.ONE_HOUR
const BUFFER_CLEANUP_INTERVAL_MS = TIMEOUTS.TEN_MINUTES
const cleanupInterval = setInterval(() => {
const now = Date.now()