Feat: add sentry integration #25
Conversation
WalkthroughThis pull request enhances the application’s error monitoring and performance tracking by integrating Sentry. Key updates include adding new Sentry dependencies in the package configuration and updating various source files to initialize Sentry, capture exceptions, and wrap critical operations with Sentry spans. Global error handlers have also been introduced to log unhandled exceptions and rejections. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application (src/index.ts)
participant Sentry as Sentry Module (src/lib/sentry.ts)
participant DB as Database Query (PacketController)
App->>Sentry: initSentry('rapid-relayer')
Sentry-->>App: Initialization complete
App->>DB: getSendPackets() (executes query)
alt SENTRY_DSN is set
DB->>Sentry: Wrap query in Sentry span
else Not set
DB-->>App: Execute query directly
end
sequenceDiagram
participant Process as Node Process
participant ErrorHandler as Global Error Handlers (setupErrorHandlers)
participant Sentry as captureException
Process->>ErrorHandler: Unhandled exception/rejection or warning
ErrorHandler->>Sentry: captureException(error)
ErrorHandler-->>Process: Schedule process exit after delay
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
src/lib/sentry.ts (2)
6-22: Allow optional handling of flush failures.
Currently, flush failures are silently ignored. Consider logging or handling the error fromSentry.flush()to aid in debugging network or server issues and to confirm that the event was indeed sent to Sentry.await Sentry.flush(1000).catch(() => undefined) + // Optionally: + /* + await Sentry.flush(1000).catch((err) => { + console.error("Sentry flush failed:", err); + }); + */
24-69: Avoid logging Sentry DSN for security reasons.
Exposing the DSN in logs could allow unauthorized event submissions. Consider masking it or omitting it entirely for better security practices.console.log(`Sentry initialized: server_name: ${serverName} - dsn: ${dsn} + dsn: [REDACTED] env: ${env || 'development'} ... `)src/lib/error-handlers.ts (3)
4-15: Ensure sufficient time for error reporting before exit.
Forcefully exiting the process withprocess.exit(1)may cut off Sentry’s flush if the system is heavily loaded. Consider a graceful shutdown strategy or a slightly longer timeout for critical systems.
17-23: Apply consistent approach for uncaught exceptions.
Same concern as unhandled rejections. A graceful shutdown or a longer delay may be preferable to ensure all resources are released properly.
25-27: Optionally capture process warnings.
If warnings correlate with relevant runtime issues, consider capturing them with Sentry for more comprehensive visibility.process.on('warning', (warning) => { console.warn('Process warning:', warning) + // Optionally capture to Sentry: + // void captureException(new Error(`Process warning: ${warning.message}`), 'warning') })src/workers/chain.ts (2)
95-96: Consider creating a utility function for error handling.The error handling pattern
e instanceof Error ? e : new Error(String(e))is repeated multiple times. Consider extracting it into a utility function for better maintainability.+// In src/lib/error.ts +export const normalizeError = (e: unknown): Error => + e instanceof Error ? e : new Error(String(e)); +// In this file -await captureException(e instanceof Error ? e : new Error(String(e))) +await captureException(normalizeError(e))
223-230: Improve error message clarity in feedEvents.The error message contains a typo and could be more descriptive.
-const errorMsg = `Fail to fecth block result. resonse - ${e}` +const errorMsg = `Failed to fetch block result. Response: ${e}`src/db/controller/packet.ts (1)
119-129: Consider centralizing Sentry configuration checks.The SENTRY_DSN environment variable check could be moved to a central configuration utility to maintain consistency across the application.
+// In src/lib/sentry.ts +export const isSentryEnabled = (): boolean => !!process.env.SENTRY_DSN; +export const withSentrySpan = <T>( + operation: string, + name: string, + fn: () => T +): T => { + if (!isSentryEnabled()) { + return fn(); + } + return Sentry.startSpan( + { + op: operation, + name, + }, + fn + ); +}; +// In this file -if (!process.env.SENTRY_DSN) { - return executeQuery() -} - -return Sentry.startSpan( - { - op: 'db.query', - name: 'getSendPackets', - }, - executeQuery -) +return withSentrySpan('db.query', 'getSendPackets', executeQuery)src/workers/wallet.ts (2)
49-51: Structure error handling consistently.The error handling could be more structured and consistent with other parts of the application.
-await captureException(e instanceof Error ? e : new Error(String(e))) -this.logger.error(`[run] ${e}`) +const error = normalizeError(e); +await captureException(error); +this.logger.error(`[run] ${error.message}`);
382-386: Improve error handling in sequence parsing.The error handling could be more informative by including the original error message.
-await captureException( - e instanceof Error ? e : new Error(String(e)) -) -this.logger.warn(`error to parse sequence`) +const error = normalizeError(e); +await captureException(error); +this.logger.warn(`Failed to parse sequence: ${error.message}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.gitignore(1 hunks)package.json(1 hunks)src/db/controller/packet.ts(2 hunks)src/index.ts(1 hunks)src/lib/error-handlers.ts(1 hunks)src/lib/sentry.ts(1 hunks)src/workers/chain.ts(5 hunks)src/workers/wallet.ts(4 hunks)
🔇 Additional comments (4)
.gitignore (2)
12-13: Clarify the/test_dbignore entry.
The modification from a negative (-/test_db) to a positive (/test_db) entry reaffirms that the test database directory will be ignored. This change maintains security by ensuring that any temporary or test-related data isn’t inadvertently committed. Please double-check that this change does not conflict with any local testing setups where the/test_dbfolder might be needed.
14-15: Protect Sensitive Data by Ignoring.env*Files.
Adding the.env*entry is a strong move to avoid accidental commits of environment variables, which are especially critical now with the Sentry integration requiring secrets likeSENTRY_DSN. This helps keep sensitive configuration secure.src/lib/sentry.ts (1)
1-4: Imports are clear and well-structured.
No concerns regarding correctness or maintainability in these lines.src/index.ts (1)
8-11: Good practice of early Sentry initialization.
CallinginitSentrybefore other setup steps ensures errors are tracked from the start. This integration looks solid.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/workers/wallet.ts (1)
394-400: Enhance error message with transaction context.The error handling could be improved by including more transaction context in the error message:
const error = new Error( - `Tx failed. raw log - ${result.raw_log}, code - ${result.code}` + `Transaction failed. Chain: ${this.chain.chainId}, Address: ${this.address()}, Code: ${result.code}, Raw Log: ${result.raw_log}` ) await captureException(error) -this.logger.error(error) +this.logger.error(`Transaction error: ${error.message}`) throw error
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/workers/wallet.ts (1)
47-61: 🛠️ Refactor suggestionEnhance retry mechanism with exponential backoff.
The current retry implementation with a fixed delay could be improved.
Apply the changes suggested in the previous review to implement exponential backoff for better reliability.
🧹 Nitpick comments (2)
src/workers/wallet.ts (2)
72-79: Consider adding rate limiting to error tracking.The current implementation tracks errors with a 10-minute cooldown, which is good. However, consider these enhancements:
- Make the cooldown duration configurable
- Add a maximum error count per error code
+const ERROR_COOLDOWN_MS = 10 * 60 * 1000 // 10 minutes +const MAX_ERRORS_PER_CODE = 100 private async checkAndStoreError(code: string, error: Error) { const now = Date.now() const lastErrorTime = this.errorTracker.get(code) ?? 0 - if (now - lastErrorTime > 10 * 60 * 1000) { + if (now - lastErrorTime > ERROR_COOLDOWN_MS) { this.errorTracker.set(code, now) + const errorCount = (this.errorTracker.get(`${code}_count`) ?? 0) + 1 + if (errorCount <= MAX_ERRORS_PER_CODE) { + this.errorTracker.set(`${code}_count`, errorCount) await captureException(error) + } } }
404-410: Consider categorizing errors by type.The error handling for transaction failures could be enhanced by categorizing errors and providing more context.
+enum TxErrorCategory { + SEQUENCE_MISMATCH = 'sequence_mismatch', + INSUFFICIENT_FUNDS = 'insufficient_funds', + UNKNOWN = 'unknown' +} +private getTxErrorCategory(rawLog: string): TxErrorCategory { + if (rawLog.startsWith('account sequence mismatch')) { + return TxErrorCategory.SEQUENCE_MISMATCH + } + if (rawLog.includes('insufficient funds')) { + return TxErrorCategory.INSUFFICIENT_FUNDS + } + return TxErrorCategory.UNKNOWN +} -const error = new Error( - `Tx failed. raw log - ${result.raw_log}, code - ${result.code}` -) +const category = this.getTxErrorCategory(result.raw_log) +const error = new Error( + `Tx failed. Category: ${category}, raw log - ${result.raw_log}, code - ${result.code}` +) -await this.checkAndStoreError(result.code.toString(), error) +await this.checkAndStoreError(`${category}_${result.code}`, error)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/workers/wallet.ts(4 hunks)
🔇 Additional comments (1)
src/workers/wallet.ts (1)
24-24: LGTM! Good addition of error tracking capabilities.The integration of Sentry and the addition of an error tracker map are well-structured additions that will improve error monitoring.
Also applies to: 30-30
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/lib/error-handlers.ts (3)
4-15: Consider making the exit delay configurable.The 1-second delay before process exit is hardcoded. Consider making this configurable to accommodate different environments and network conditions.
+const EXIT_DELAY_MS = process.env.ERROR_HANDLER_EXIT_DELAY_MS + ? parseInt(process.env.ERROR_HANDLER_EXIT_DELAY_MS, 10) + : 1000; + export function setupErrorHandlers(): void { process.on('unhandledRejection', (error: unknown) => { console.error('Unhandled rejection:', error) const errorObject = error instanceof Error ? error : new Error(typeof error === 'string' ? error : JSON.stringify(error)) void captureException(errorObject) - setTimeout(() => process.exit(1), 1000) + setTimeout(() => process.exit(1), EXIT_DELAY_MS) })
17-23: Update the exit delay to use the configurable value.Apply the same configurable exit delay here for consistency.
process.on('uncaughtException', (error) => { console.error('Uncaught exception:', error) void captureException(error) - setTimeout(() => process.exit(1), 1000) + setTimeout(() => process.exit(1), EXIT_DELAY_MS) })
3-24: Consider implementing graceful shutdown.While the error handling is solid, consider implementing a graceful shutdown strategy:
- Clean up resources (close database connections, file handles)
- Complete in-flight requests
- Stop accepting new requests
- Notify load balancers/health checks
This ensures proper cleanup and minimizes impact during error scenarios.
Would you like me to provide an example implementation of a graceful shutdown handler?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/error-handlers.ts(1 hunks)
🔇 Additional comments (2)
src/lib/error-handlers.ts (2)
7-11: LGTM! Robust error object handling.The code properly handles different error types by converting them to Error instances, which is crucial for consistent error reporting in Sentry.
1-3:✅ Verification successful
Verify the Sentry module implementation.
The code imports
captureExceptionfrom a local Sentry module. Let's verify its implementation to ensure proper error reporting.
🏁 Script executed:
#!/bin/bash # Description: Verify the Sentry module implementation # Check if the sentry.ts file exists and contains the captureException implementation fd -e ts -E "node_modules" "sentry.ts" --exec cat {}Length of output: 1816
Sentry Module Integration Verified
The
captureExceptionimplementation in the local Sentry module is correctly configured to report errors:
- It appropriately checks for the
SENTRY_DSNenvironment variable before attempting reporting.- The error is captured within a Sentry scope with the specified severity level.
- A flush is performed (with a 1-second timeout) to ensure events are sent.
There are no issues with the Sentry integration, and the usage in
setupErrorHandlers()aligns with our intended error reporting behavior.
Summary by CodeRabbit
New Features
Chores
Dependencies