Conversation
WalkthroughEnables Sentry log collection in initialization and augments error paths in lightning swap handling to log failures and swap context to Sentry before preserving existing error handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying wallet-signet with
|
| Latest commit: |
701f1c2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f1760196.wallet-23u.pages.dev |
| Branch Preview URL: | https://sentry-logs.wallet-23u.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
701f1c2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://43af6ef9.arkade-wallet.pages.dev |
| Branch Preview URL: | https://sentry-logs.arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
701f1c2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1ced07d8.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://sentry-logs.wallet-bitcoin.pages.dev |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.tsx(1 hunks)src/lib/lightning.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/lightning.ts (1)
src/lib/logs.ts (1)
consoleError(40-44)
🔇 Additional comments (2)
src/lib/lightning.ts (2)
16-16: LGTM: Sentry import added.The import is correctly added to support error logging to Sentry.
163-165: The core claim in the original review comment is factually incorrect.Sentry.logger is a valid public API in React SDK v9.x that exposes logging methods: trace, debug, info, warn, error, fatal, and the codebase has Sentry initialized with enableLogs: true (src/index.tsx line 25), so the implementation is using a documented public API correctly—not an internal logger.
However, there are two minor valid concerns:
Log Level: Using
infofor a failed refund is semantically inappropriate. The condition represents an error, sowarnorerrorlevel would be more suitable.Data Exposure: Logging the entire
swapobject may include sensitive fields. Since sendDefaultPii: false is configured, and the swap type is from an external library (@arkade-os/boltz-swap), consider logging only the swap ID and error details rather than the full object.The suggestion to use
captureExceptioninstead is a valid alternative approach but not a correction of incorrect API usage.Likely an incorrect or invalid review comment.
| Sentry.init({ | ||
| dsn: sentryDsn, | ||
| sendDefaultPii: false, | ||
| enableLogs: true, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the scope and impact of enabling log collection.
The enableLogs: true configuration will automatically capture console logs (console.log, console.warn, console.error, etc.) as breadcrumbs in Sentry. While this provides helpful debugging context, verify that:
- This doesn't inadvertently capture sensitive information logged to console
- The volume of logs won't cause performance issues or exceed Sentry quotas
- This aligns with your privacy policy (though
sendDefaultPii: falseis correctly set)
🌐 Web query:
Sentry React enableLogs configuration implications
💡 Result:
Short summary — what setting enableLogs does and the main implications:
- What it does: enables Sentry’s Structured Logs feature so your app can send log entries (trace/debug/info/warn/error/fatal) via Sentry.logger and related APIs (and the consoleLoggingIntegration). [1][2][3].
- How to enable: JavaScript SDKs use enableLogs (or in some older/react-native v7 betas _experiments.enableLogs) in Sentry.init options. SDK version requirements vary by platform (examples: JS logs supported in recent 9.x SDKs; check your SDK docs). [1][4][5].
- Filtering & control: you can filter/modify or drop logs client‑side with beforeSendLog / beforeSendLogs and with integrations (consoleLoggingIntegration, transports, level filters). Use these to strip PII and reduce noise. [3][6].
- Costs & quota: logs count toward your Logs quota/usage (plans include a baseline e.g. 5 GB/month; extra usage may incur PAYG charges). Monitor Stats & Usage and disable if you don’t want logs consuming budget. [7][8].
- Privacy & volume risk: enabling logs may send high volumes and sensitive data — review and apply filtering, sampling, and level restrictions before turning it on in production. [3][6][8].
- Operational notes: Logs were gated/experimental in earlier releases (some SDKs used _experiments.enableLogs); Sentry has moved Logs to generally available but verify your SDK version and the correct option name in your platform docs. [5][1][4].
If you want, tell me your @sentry/* SDK package and version and I’ll give the exact init config, recommended beforeSendLog example to strip PII, and how to monitor quota.
Sources: Sentry docs (Set Up Logs, SDK options, consoleLoggingIntegration), Sentry release notes and pricing/help articles. [1][2][3][4][5][6][7][8]
References
- Sentry docs — Set Up Logs (JS/React variants).
- Sentry docs — SDK Options (enableLogs).
- Sentry docs — consoleLoggingIntegration & beforeSendLog examples.
- Sentry @sentry/react release notes (logs support / _experiments).
- @sentry/react changelog indicating logs support.
- Sentry docs — structured logs usage and filtering.
- Sentry pricing & quotas.
- Sentry Help Center — disable Structured Logging / usage guidance.
I need to verify a few more details in the codebase to ensure the enableLogs configuration is properly safeguarded. Let me check for filtering and quota management:
🏁 Script executed:
#!/bin/bash
# Check for Sentry SDK version and any logging filtering configuration
echo "=== Checking package.json for Sentry SDK version ==="
cat package.json | grep -A5 '"@sentry'
echo -e "\n=== Checking for beforeSendLog filtering ==="
rg -n "beforeSendLog" --type ts --type tsx --type js
echo -e "\n=== Checking for log integration or logging level configuration ==="
rg -n "consoleLoggingIntegration|integrations|logLevel" --type ts --type tsx -C2
echo -e "\n=== Checking full Sentry.init configuration ==="
rg -n "Sentry\.init" --type ts --type tsx -A10Length of output: 476
I need to retry with corrected syntax to see the full Sentry configuration:
🏁 Script executed:
#!/bin/bash
# Search with broader file type matching
echo "=== Full Sentry.init configuration ==="
rg -n "Sentry\.init" -A15
echo -e "\n=== Checking for beforeSendLog ==="
rg -n "beforeSendLog"
echo -e "\n=== Checking integrations array ==="
rg -n "integrations" -B2 -A3Length of output: 742
Add log filtering and level restrictions before production deployment.
The enableLogs: true configuration is enabled without filtering or level controls. Per Sentry's documentation, logs are recommended to be filtered before production to prevent PII capture and quota overages.
Add a beforeSendLog callback to filter sensitive data:
Sentry.init({
dsn: sentryDsn,
sendDefaultPii: false,
enableLogs: true,
beforeSendLog: (log) => {
// Filter sensitive data and apply level restrictions
return log;
},
})Alternatively, configure the consoleLoggingIntegration with level restrictions to limit log volume and sensitivity.
🤖 Prompt for AI Agents
In src/index.tsx around line 25, the Sentry config sets enableLogs: true without
any filtering or level controls; add a beforeSendLog callback that inspects each
log event, drops or redacts sensitive fields (PII, tokens, user data) and
rejects events below a configured minimum level (e.g., only allow warn+ in
production), or alternatively configure the consoleLoggingIntegration with level
restrictions and sampling to limit volume; implement environment-aware behavior
(stricter in production), ensure sendDefaultPii is false, and return null from
beforeSendLog for events you want to drop and the (possibly redacted) log object
for events you keep.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/lightning.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/lightning.ts (1)
src/lib/logs.ts (1)
consoleError(40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
src/lib/lightning.ts (2)
16-16: LGTM!The Sentry import is correctly added.
95-97: Verify swap object contents and establish Sentry logging guidelines.Before merging, please:
Review swap object structure: Examine
PendingReverseSwapandPendingSubmarineSwaptypes to identify any PII or sensitive data (user identifiers, email addresses, payment details, etc.) that should not be logged to Sentry.Establish logging guidelines: Consider documenting which fields are safe to log and creating a helper function to sanitize swap objects before logging, ensuring consistent privacy practices across the codebase.
Sentry configuration: Confirm that the Sentry configuration includes appropriate
beforeSendhooks to scrub sensitive data if needed.Run this script to examine the swap type definitions:
Also applies to: 128-130, 164-166
| } catch (error) { | ||
| Sentry.logger.info(`Failed to claim swap ${pendingSwap.response.id}`, { error, swap: pendingSwap }) | ||
| throw this.someError(error, 'Error claiming VHTLC') |
There was a problem hiding this comment.
Use appropriate log level and review sensitive data exposure.
Two concerns with the Sentry logging:
-
Log level: Using
Sentry.logger.info()for error scenarios may cause these failures to be overlooked in monitoring. Errors should useerrorlevel orcaptureException(). -
Sensitive data: The full
pendingSwapobject is logged, which may contain PII or sensitive information (addresses, amounts, user identifiers). Review whether this data should be sanitized or if only specific fields should be logged.
Consider using Sentry.captureException() for better error tracking with stack traces:
- Sentry.logger.info(`Failed to claim swap ${pendingSwap.response.id}`, { error, swap: pendingSwap })
+ Sentry.captureException(error, {
+ extra: {
+ swapId: pendingSwap.response.id,
+ // Add only non-sensitive swap details here
+ }
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| Sentry.logger.info(`Failed to claim swap ${pendingSwap.response.id}`, { error, swap: pendingSwap }) | |
| throw this.someError(error, 'Error claiming VHTLC') | |
| } catch (error) { | |
| Sentry.captureException(error, { | |
| extra: { | |
| swapId: pendingSwap.response.id, | |
| // Add only non-sensitive swap details here | |
| } | |
| }) | |
| throw this.someError(error, 'Error claiming VHTLC') |
🤖 Prompt for AI Agents
In src/lib/lightning.ts around lines 95-97, replace the Sentry.logger.info call
with an error-level capture and avoid logging the entire pendingSwap object;
call Sentry.captureException (or Sentry.logger.error) with the caught error and
pass a sanitized context object containing only non-sensitive identifiers (e.g.,
swap id, minimal status or error codes) rather than the full pendingSwap payload
to prevent PII exposure. Ensure the thrown error remains wrapped with
this.someError as before.
| } catch (error) { | ||
| Sentry.logger.info(`Failed to refund swap ${pendingSwap.response.id}`, { error, swap: pendingSwap }) | ||
| throw this.someError(error, 'Swap failed: VHTLC refund failed') |
There was a problem hiding this comment.
Same issues: log level and sensitive data exposure.
This has the same concerns as the waitAndClaim error logging:
- Using
infolevel for error scenarios - Logging the full
pendingSwapobject may expose sensitive data
Apply a similar fix as suggested for line 96, using Sentry.captureException() with sanitized context:
- Sentry.logger.info(`Failed to refund swap ${pendingSwap.response.id}`, { error, swap: pendingSwap })
+ Sentry.captureException(error, {
+ extra: {
+ swapId: pendingSwap.response.id,
+ // Add only non-sensitive swap details here
+ }
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| Sentry.logger.info(`Failed to refund swap ${pendingSwap.response.id}`, { error, swap: pendingSwap }) | |
| throw this.someError(error, 'Swap failed: VHTLC refund failed') | |
| } catch (error) { | |
| Sentry.captureException(error, { | |
| extra: { | |
| swapId: pendingSwap.response.id, | |
| // Add only non-sensitive swap details here | |
| } | |
| }) | |
| throw this.someError(error, 'Swap failed: VHTLC refund failed') |
🤖 Prompt for AI Agents
In src/lib/lightning.ts around lines 128 to 130, the catch block currently logs
errors with Sentry.logger.info and includes the full pendingSwap object; change
this to use Sentry.captureException(error, { level: 'error', extra: { swapId:
pendingSwap?.response?.id } }) (or include only non-sensitive fields you
explicitly allow), remove the full pendingSwap from logs to avoid exposing
sensitive data, and then rethrow using the existing this.someError(error, 'Swap
failed: VHTLC refund failed') so behavior is preserved.
| } catch (error) { | ||
| Sentry.logger.info(`Failed to refund swap ${swap.response.id}`, { error, swap }) | ||
| consoleError(error, `Failed to refund swap ${swap.response.id}`) |
There was a problem hiding this comment.
Address log level, sensitive data, and potential duplicate logging.
Three concerns here:
-
Log level: Same issue - using
info()for error scenarios. -
Sensitive data: The full
swapobject is logged to Sentry. -
Duplicate logging: Both
Sentry.logger.info()andconsoleError()are called. SinceconsoleError()already logs to console and internal logs (persrc/lib/logs.ts), this creates redundant log entries. This may be intentional to preserve existing behavior, but worth confirming.
Consider consolidating or clarifying the logging strategy:
- Sentry.logger.info(`Failed to refund swap ${swap.response.id}`, { error, swap })
- consoleError(error, `Failed to refund swap ${swap.response.id}`)
+ Sentry.captureException(error, {
+ extra: {
+ swapId: swap.response.id,
+ // Add only non-sensitive swap details here
+ }
+ })
+ consoleError(error, `Failed to refund swap ${swap.response.id}`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| Sentry.logger.info(`Failed to refund swap ${swap.response.id}`, { error, swap }) | |
| consoleError(error, `Failed to refund swap ${swap.response.id}`) | |
| } catch (error) { | |
| Sentry.captureException(error, { | |
| extra: { | |
| swapId: swap.response.id, | |
| // Add only non-sensitive swap details here | |
| } | |
| }) | |
| consoleError(error, `Failed to refund swap ${swap.response.id}`) |
🤖 Prompt for AI Agents
In src/lib/lightning.ts around lines 164-166, replace the Sentry.logger.info +
consoleError duplicate logging with a single consolidated error report: remove
the Sentry.logger.info call, keep the existing consoleError(error, `Failed to
refund swap ${swap.response.id}`) for local/internal logs, and separately send
the error to Sentry via Sentry.captureException(error, { level: 'error', extra:
{ swapId: swap.response.id } }) (do not include the full swap object or other
sensitive fields; only include non-sensitive identifiers like swap.response.id).
2 participants
@louisinger please review
Summary by CodeRabbit