feat: add logging option to Braintree payment provider for enhanced d…

[lcmohsen]

Add a feature to help developers debug with more detailed errors in their logs.

• Introduced a logging option in Braintree options to enable console logging for important operations.
• Implemented logging functionality in both BraintreeBase and BraintreeImport classes to capture debug information and error details.
• Updated README.md to document the new logging feature.

Summary by CodeRabbit

• New Features
  • Added an optional debug logging flag for the Braintree payment provider. When enabled, detailed runtime logs are produced across payment flows (initialization, transactions, authorization, capture, refund, cancel, webhooks). Logging is disabled by default in production and enabled in non-production; toggle via the provider configuration option.

[coderabbitai]

Walkthrough

Adds an optional boolean logging option to the Braintree payment provider and instruments internal flows with conditional debug and structured error logging, enabled by default when NODE_ENV is not production and disabled in production.

Changes

Cohort / File(s)	Summary
Documentation plugins/braintree-payment/README.md	Adds description for the new logging boolean option and its default behavior (enabled outside production).
Type Definitions plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts	Adds optional logging?: boolean to the exported BraintreeOptions interface.
Core Implementation plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts, plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts	Adds protected/private logging helpers (logDebug, logErrorDetail), includes logging in option validation, and emits conditional debug and structured error logs across payment lifecycle methods (initiate, authorize, capture, refund, cancel, retrieve, account holder/webhook handling). No public API signature changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding a logging option to the Braintree payment provider. This is the primary focus of the changeset across all modified files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/add-logging-flag

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 964-967: The error log in createBraintreeCustomer is including
customer.email (PII); update the call to logErrorDetail in braintree-base.ts so
the context uses a non-PII identifier (e.g., customer.id or customerId) instead
of customer.email and keep the errors payload intact (refer to the
logErrorDetail call and the customerResult/errors objects to locate the change).
- Line 825: The debug log in createAccountHolder currently outputs the
customer's email via this.logDebug('createAccountHolder', { customerEmail:
input.context.customer?.email }); which exposes PII; change it to avoid logging
the raw email by either removing the customerEmail field or replacing it with a
non-identifying masked value (e.g., hash or partial mask like first/last char +
****) before calling this.logDebug so the log call no longer contains the full
email.

🧹 Nitpick comments (2)

plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)

146-154: Consider using logger.warn for error details instead of logger.info.

The logErrorDetail method logs error information (including stack traces) at info level. While this is gated behind the logging option and meant for debugging, using logger.warn or logger.error would make it easier to filter and alert on these entries in log aggregation systems.

💡 Proposed change

   protected logErrorDetail(operation: string, error: unknown, context?: Record<string, unknown>): void {
     if (!this.options_.logging) return;
     const msg = error instanceof Error ? error.message : String(error);
     const stack = error instanceof Error ? error.stack : undefined;
     const ctx = context ? ` ${JSON.stringify(context)}` : '';
     const stackLine = stack ? ` stack: ${stack}` : '';
-    this.logger.info(`[Braintree] ERROR ${operation}: ${msg}${ctx}${stackLine}`);
+    this.logger.warn(`[Braintree] ERROR ${operation}: ${msg}${ctx}${stackLine}`);
   }

plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts (1)

88-100: Duplicated logging helpers with inconsistent implementation.

The logDebug and logErrorDetail methods are duplicated from BraintreeBase. Additionally, logErrorDetail here is missing the stack trace logging that exists in the base class implementation. Consider extracting these to a shared utility function to ensure consistency and reduce duplication.

♻️ Add missing stack trace and consider extraction

At minimum, align the implementation with braintree-base.ts:

   private logErrorDetail(operation: string, error: unknown, context?: Record<string, unknown>): void {
     if (!this.options.logging) return;
     const msg = error instanceof Error ? error.message : String(error);
+    const stack = error instanceof Error ? error.stack : undefined;
     const ctx = context ? ` ${JSON.stringify(context)}` : '';
-    this.logger.info(`[Braintree Import] ERROR ${operation}: ${msg}${ctx}`);
+    const stackLine = stack ? ` stack: ${stack}` : '';
+    this.logger.info(`[Braintree Import] ERROR ${operation}: ${msg}${ctx}${stackLine}`);
   }

Ideally, extract both helpers to a shared utility:

// utils/logging.ts
export const createLogDebug = (logger: Logger, prefix: string, enabled: boolean) =>
  (message: string, context?: Record<string, unknown>) => {
    if (!enabled) return;
    const msg = context ? `${message} ${JSON.stringify(context)}` : message;
    logger.info(`${prefix} ${msg}`);
  };

[antony-lambdacurry]

@currybot-lc review

[currybot-lc]

🍛 Currybot Review — PR #32

feat: add logging option to Braintree payment provider

Summary

Adds an opt-in logging boolean to BraintreeOptions. When enabled, logDebug and logErrorDetail helper methods emit structured info via this.logger.info() across all payment operations (initiate, authorize, capture, refund, cancel, webhooks, account holder CRUD). Docs updated. Default is false.

Overall Assessment

Clean, well-scoped PR. The pattern is consistent and the coverage across operations is thorough. A few things worth considering:

---

Issues

1. logErrorDetail uses logger.info() — should be logger.error() or logger.warn()

Both BraintreeBase and BraintreeImport log error details at info level:

this.logger.info(`[Braintree] ERROR ${operation}: ${msg}${ctx}${stackLine}`);

Even behind a debug flag, error-level context should use logger.error() (or at minimum logger.warn()). This matters for log aggregation/filtering — teams searching for errors in production logs won't find these at info level.

Severity: Medium — functional but will cause confusion in log analysis.

2. BraintreeImport.logErrorDetail drops the stack trace

BraintreeBase.logErrorDetail includes error.stack, but BraintreeImport.logErrorDetail does not:

// BraintreeBase (has stack)
const stackLine = stack ? ` stack: ${stack}` : '';

// BraintreeImport (no stack)
this.logger.info(`[Braintree Import] ERROR ${operation}: ${msg}${ctx}`);

If both classes should behave the same, the Import version should include the stack too. Consider extracting this into a shared utility or having BraintreeImport extend BraintreeBase for these helpers (if it doesn't already).

Severity: Low — inconsistency, not a bug.

3. Duplicated logDebug / logErrorDetail across two classes

BraintreeBase and BraintreeImport each define their own copies of logDebug and logErrorDetail with slightly different implementations (see #2). This is a DRY opportunity — a shared mixin, base class method, or standalone utility function would prevent drift.

Severity: Low — maintenance concern.

4. Minor: error message improvement in initiatePayment

Nice catch improving the error message from just gatewayRejectionReason to gatewayRejectionReason ?? saleResponse.message ?? 'Unknown error'. That's a real improvement for debugging failed sales — 👍.

---

Merge Confidence: High ✅

This is a safe, additive change. The logging is properly gated behind an opt-in flag, defaults to false, and doesn't change any control flow. The issues above are all improvement suggestions, not blockers.

Ship it — consider the info → error level change and the DRY cleanup as a fast follow-up.