Skip to content

refactor: improve error handling in createTransaction #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: improve error handling in createTransaction #28

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 44 additions & 35 deletions plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,17 @@ const validateOptionalString = (value: unknown, fieldName: string): string | und
};

// Error handling utility that preserves full error context
export const buildBraintreeError = (error: unknown, operation: string, logger: Logger, context?: Record<string, unknown>): MedusaError => {
export const buildBraintreeError = (
error: unknown,
operation: string,
logger: Logger,
context?: Record<string, unknown>,
): MedusaError => {
const errorMessage = error instanceof Error ? error.message : String(error);

// Preserve full error context in logging
logger.error(`Braintree ${operation} failed: ${errorMessage}`, error instanceof Error ? error : undefined);

return new MedusaError(MedusaError.Types.INVALID_DATA, `Failed to ${operation}: ${errorMessage}`);
};

Expand Down Expand Up @@ -216,7 +221,6 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
}

private formatToTwoDecimalStringIfFinite(amount: unknown): string | undefined {

const n = Number(amount);
if (!Number.isFinite(n)) return undefined;
return formatToTwoDecimalString(n);
Expand Down Expand Up @@ -337,10 +341,15 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
if (!sessionData.payment_method_nonce)
throw new MedusaError(MedusaError.Types.INVALID_ARGUMENT, 'Payment method nonce is required');

if (!transaction)
transaction = await this.createTransaction({
input,
});
if (!transaction) {
try {
transaction = await this.createTransaction({
input,
});
} catch (error) {
throw error;
}
}
Comment on lines +344 to +352
Copy link

@coderabbitai coderabbitai bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove redundant try-catch block.

The inner try-catch block that immediately rethrows adds no value since the outer try-catch at line 335 already handles all errors from this function, including those from createTransaction.

Apply this diff to simplify the error handling:

       if (!transaction) {
-        try {
-          transaction = await this.createTransaction({
-            input,
-          });
-        } catch (error) {
-          throw error;
-        }
+        transaction = await this.createTransaction({
+          input,
+        });
       }
📝 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.

Suggested change
if (!transaction) {
try {
transaction = await this.createTransaction({
input,
});
} catch (error) {
throw error;
}
}
if (!transaction) {
transaction = await this.createTransaction({
input,
});
}
🤖 Prompt for AI Agents 
In
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
around lines 344 to 352, remove the redundant inner try-catch that simply
rethrows the caught error; instead directly await this.createTransaction({ input
}) to assign transaction so the outer try-catch at ~line 335 handles any errors,
keeping behavior identical but simplifying the code and eliminating unnecessary
exception wrapping.


const paymentStatusRequest: GetPaymentStatusInput = {
...input,
Expand All @@ -363,7 +372,7 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
};
} catch (error) {
this.logger.error(`Error authorizing transaction: ${error.message}`, error);
throw new MedusaError(MedusaError.Types.INVALID_DATA, `Failed to authorize transaction`);
throw new MedusaError(MedusaError.Types.INVALID_DATA, error.message ?? 'Unknown error');
}
}

Expand Down Expand Up @@ -442,14 +451,14 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
countryCodeAlpha2: this.sanitizeCountryCodeAlpha2(context.shipping_address?.country_code),
}
: undefined;
let cart: CartDTO | undefined;
if(context.items?.[0]?.cart_id) {
cart = await this.cartService.retrieveCart(context.items?.[0]?.cart_id as string);
if (!cart) {
throw new MedusaError(MedusaError.Types.NOT_FOUND, 'Cart not found');
}
}
let cart: CartDTO | undefined;
if (context.items?.[0]?.cart_id) {
cart = await this.cartService.retrieveCart(context.items?.[0]?.cart_id as string);
if (!cart) {
throw new MedusaError(MedusaError.Types.NOT_FOUND, 'Cart not found');
}
}

const lineItems = context.items
?.slice(0, 249)
.map((item) => {
Expand All @@ -464,8 +473,8 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
kind: 'debit',
name: name.substring(0, 127), // Max 127 characters
productCode: ((item.metadata?.productCode as string) ?? item.product_id).substring(0, 12), // Max 12 characters
commodityCode: ((item.metadata?.commodityCode as string) ?? "placeholder").substring(0, 12), // Max 12 characters
description: (item.product_description ?? "Healthcare").substring(0, 127), // Max 127 characters
commodityCode: ((item.metadata?.commodityCode as string) ?? 'placeholder').substring(0, 12), // Max 12 characters
description: (item.product_description ?? 'Healthcare').substring(0, 127), // Max 127 characters
url: `/${item.product_handle}`.substring(0, 255), // Max 255 characters
unitOfMeasure: ((cart?.currency_code as string)?.toUpperCase() ?? 'USD').substring(0, 12), // Max 12 characters
taxAmount: this.formatToTwoDecimalStringIfFinite(Number(item.tax_total)), // Must be decimal string
Expand Down Expand Up @@ -552,14 +561,16 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
const schema = z.object({
paymentMethodNonce: z.string().optional(),
payment_method_nonce: z.string().optional(),
cardDetails:z.object({
cardType: z.string().optional(),
lastFour: z.string().optional(),
lastTwo: z.string().optional(),
expirationMonth: z.string().optional(),
expirationYear: z.string().optional(),
cardholderName: z.string().optional(),
}).optional(),
cardDetails: z
.object({
cardType: z.string().optional(),
lastFour: z.string().optional(),
lastTwo: z.string().optional(),
expirationMonth: z.string().optional(),
expirationYear: z.string().optional(),
cardholderName: z.string().optional(),
})
.optional(),
});

const result = schema.safeParse(data);
Expand Down Expand Up @@ -626,7 +637,7 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
if (!saleResponse.success) {
throw new MedusaError(
MedusaError.Types.PAYMENT_AUTHORIZATION_ERROR,
`Failed to create Braintree transaction: ${JSON.stringify(saleResponse)}`,
saleResponse.transaction.gatewayRejectionReason ?? 'Unknown error',
);
}

Expand All @@ -636,7 +647,9 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
if (saleResponse.transaction?.id) {
await this.gateway.transaction.void(saleResponse.transaction.id);
}
throw buildBraintreeError(error, 'sync payment session', this.logger, { transactionId: saleResponse.transaction?.id });
throw buildBraintreeError(error, 'sync payment session', this.logger, {
transactionId: saleResponse.transaction?.id,
});
}
} catch (error) {
throw buildBraintreeError(error, 'create Braintree transaction', this.logger);
Expand All @@ -660,7 +673,6 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
} catch (e) {
throw buildBraintreeError(e, 'delete Braintree payment', this.logger);
}

} else {
return {
data: {
Expand Down Expand Up @@ -734,12 +746,12 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {

let shouldVoid = ['submitted_for_settlement', 'authorized'].includes(transaction.status);

if(process.env.TEST_FORCE_SETTLED === 'true') {
if (process.env.TEST_FORCE_SETTLED === 'true') {
shouldVoid = false;
await this.gateway.testing.settle(transaction.id);
transaction = await this.retrieveTransaction(transaction.id);
}

if (shouldVoid) {
const voidResponse = await this.gateway.transaction.void(transaction.id);

Expand Down Expand Up @@ -785,8 +797,6 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
`Failed to create Braintree refund: ${refundResponse.message}`,
);



const refundResult: RefundPaymentOutput = {
data: {
...input.data,
Expand Down Expand Up @@ -896,7 +906,6 @@ class BraintreeBase extends AbstractPaymentProvider<BraintreeOptions> {
} catch (e) {
throw buildBraintreeError(e, 'delete account holder', this.logger);
}

}

async getWebhookActionAndData(webhookData: ProviderWebhookPayload['payload']): Promise<WebhookActionResult> {
Expand Down