From 860b6d6160915e54bf4d68d365ff24d718f25683 Mon Sep 17 00:00:00 2001 From: jackiettran <41605212+jackiettran@users.noreply.github.com> Date: Sat, 10 Jan 2026 13:29:09 -0500 Subject: [PATCH] Stripe error handling and now you can rent an item for a different time while having an upcoming or active rental --- ...0260109000001-add-payment-failed-reason.js | 15 ++ backend/models/Rental.js | 3 + backend/routes/rentals.js | 7 +- backend/services/stripeService.js | 2 +- .../tests/unit/services/stripeService.test.js | 4 +- backend/tests/unit/utils/stripeErrors.test.js | 107 ++----------- backend/utils/stripeErrors.js | 150 ++++++++++++------ frontend/src/pages/ItemDetail.tsx | 80 ++++------ frontend/src/pages/Renting.tsx | 5 +- frontend/src/types/index.ts | 1 + 10 files changed, 178 insertions(+), 196 deletions(-) create mode 100644 backend/migrations/20260109000001-add-payment-failed-reason.js diff --git a/backend/migrations/20260109000001-add-payment-failed-reason.js b/backend/migrations/20260109000001-add-payment-failed-reason.js new file mode 100644 index 0000000..4493467 --- /dev/null +++ b/backend/migrations/20260109000001-add-payment-failed-reason.js @@ -0,0 +1,15 @@ +"use strict"; + +module.exports = { + up: async (queryInterface, Sequelize) => { + // Add paymentFailedReason - stores the user-friendly error message for payment failures + await queryInterface.addColumn("Rentals", "paymentFailedReason", { + type: Sequelize.TEXT, + allowNull: true, + }); + }, + + down: async (queryInterface, Sequelize) => { + await queryInterface.removeColumn("Rentals", "paymentFailedReason"); + }, +}; diff --git a/backend/models/Rental.js b/backend/models/Rental.js index 1feeb86..265e972 100644 --- a/backend/models/Rental.js +++ b/backend/models/Rental.js @@ -172,6 +172,9 @@ const Rental = sequelize.define("Rental", { paymentFailedNotifiedAt: { type: DataTypes.DATE, }, + paymentFailedReason: { + type: DataTypes.TEXT, + }, // Payment method update rate limiting paymentMethodUpdatedAt: { type: DataTypes.DATE, diff --git a/backend/routes/rentals.js b/backend/routes/rentals.js index 59d5af2..b3886b0 100644 --- a/backend/routes/rentals.js +++ b/backend/routes/rentals.js @@ -654,8 +654,11 @@ router.put("/:id/status", authenticateToken, async (req, res) => { ? paymentError.renterMessage : "Your payment could not be processed. Please try a different payment method."; - // Track payment failure timestamp - await rental.update({ paymentFailedNotifiedAt: new Date() }); + // Track payment failure timestamp and reason + await rental.update({ + paymentFailedNotifiedAt: new Date(), + paymentFailedReason: renterMessage, + }); // Auto-send payment declined email to renter try { diff --git a/backend/services/stripeService.js b/backend/services/stripeService.js index f0e9d21..d0cece5 100644 --- a/backend/services/stripeService.js +++ b/backend/services/stripeService.js @@ -401,7 +401,7 @@ class StripeService { try { const session = await stripe.checkout.sessions.create({ customer: customerId, - payment_method_types: ["card", "us_bank_account", "link"], + payment_method_types: ["card", "link"], mode: "setup", ui_mode: "embedded", redirect_on_completion: "never", diff --git a/backend/tests/unit/services/stripeService.test.js b/backend/tests/unit/services/stripeService.test.js index a11236f..decd74b 100644 --- a/backend/tests/unit/services/stripeService.test.js +++ b/backend/tests/unit/services/stripeService.test.js @@ -882,7 +882,7 @@ describe('StripeService', () => { expect(mockStripeCheckoutSessionsCreate).toHaveBeenCalledWith({ customer: 'cus_123456789', - payment_method_types: ['card', 'us_bank_account', 'link'], + payment_method_types: ['card', 'link'], mode: 'setup', ui_mode: 'embedded', redirect_on_completion: 'never', @@ -915,7 +915,7 @@ describe('StripeService', () => { expect(mockStripeCheckoutSessionsCreate).toHaveBeenCalledWith({ customer: 'cus_123456789', - payment_method_types: ['card', 'us_bank_account', 'link'], + payment_method_types: ['card', 'link'], mode: 'setup', ui_mode: 'embedded', redirect_on_completion: 'never', diff --git a/backend/tests/unit/utils/stripeErrors.test.js b/backend/tests/unit/utils/stripeErrors.test.js index 34d9bae..ac4ab6d 100644 --- a/backend/tests/unit/utils/stripeErrors.test.js +++ b/backend/tests/unit/utils/stripeErrors.test.js @@ -21,42 +21,6 @@ describe('Stripe Errors Utility', () => { } }); - describe('ACH decline codes', () => { - const achCodes = [ - 'bank_account_closed', - 'bank_account_frozen', - 'bank_account_restricted', - 'bank_account_unusable', - 'bank_account_invalid_details', - 'debit_not_authorized', - ]; - - test.each(achCodes)('%s exists in DECLINE_MESSAGES', (code) => { - expect(DECLINE_MESSAGES).toHaveProperty(code); - }); - - test.each(achCodes)('%s has all required properties', (code) => { - const messages = DECLINE_MESSAGES[code]; - expect(messages).toHaveProperty('ownerMessage'); - expect(messages).toHaveProperty('renterMessage'); - expect(messages).toHaveProperty('canOwnerRetry'); - expect(messages).toHaveProperty('requiresNewPaymentMethod'); - }); - - test.each(achCodes)('%s requires new payment method', (code) => { - expect(DECLINE_MESSAGES[code].requiresNewPaymentMethod).toBe(true); - }); - - test.each(achCodes)('%s does not allow owner retry', (code) => { - expect(DECLINE_MESSAGES[code].canOwnerRetry).toBe(false); - }); - - test.each(achCodes)('%s has non-empty messages', (code) => { - expect(DECLINE_MESSAGES[code].ownerMessage.length).toBeGreaterThan(0); - expect(DECLINE_MESSAGES[code].renterMessage.length).toBeGreaterThan(0); - }); - }); - describe('Additional card decline codes', () => { const additionalCardCodes = [ 'call_issuer', @@ -189,51 +153,6 @@ describe('Stripe Errors Utility', () => { expect(result.canOwnerRetry).toBe(true); }); - test('parses ACH decline error - bank_account_closed', () => { - const error = { - type: 'StripeCardError', - code: 'card_declined', - decline_code: 'bank_account_closed', - message: 'The bank account has been closed.', - }; - - const result = parseStripeError(error); - - expect(result.code).toBe('bank_account_closed'); - expect(result.ownerMessage).toBe("The renter's bank account has been closed."); - expect(result.renterMessage).toBe('Your bank account has been closed. Please add a different payment method.'); - expect(result.requiresNewPaymentMethod).toBe(true); - }); - - test('parses ACH decline error - bank_account_frozen', () => { - const error = { - type: 'StripeCardError', - code: 'card_declined', - decline_code: 'bank_account_frozen', - message: 'The bank account is frozen.', - }; - - const result = parseStripeError(error); - - expect(result.code).toBe('bank_account_frozen'); - expect(result.ownerMessage).toBe("The renter's bank account is frozen."); - expect(result.renterMessage).toContain('frozen'); - }); - - test('parses ACH decline error - debit_not_authorized', () => { - const error = { - type: 'StripeCardError', - code: 'card_declined', - decline_code: 'debit_not_authorized', - message: 'Debit not authorized.', - }; - - const result = parseStripeError(error); - - expect(result.code).toBe('debit_not_authorized'); - expect(result.renterMessage).toContain('not authorized'); - }); - test('returns default error for unknown decline code', () => { const error = { type: 'StripeCardError', @@ -279,7 +198,7 @@ describe('Stripe Errors Utility', () => { const error = { type: 'StripeCardError', code: 'card_declined', - decline_code: 'bank_account_frozen', + decline_code: 'insufficient_funds', message: 'Original Stripe message', }; @@ -293,11 +212,11 @@ describe('Stripe Errors Utility', () => { describe('PaymentError', () => { test('creates PaymentError with all properties', () => { const parsedError = { - code: 'bank_account_frozen', - ownerMessage: "The renter's bank account is frozen.", - renterMessage: 'Your bank account is frozen.', + code: 'insufficient_funds', + ownerMessage: "The renter's card has insufficient funds.", + renterMessage: 'Your card has insufficient funds.', canOwnerRetry: false, - requiresNewPaymentMethod: true, + requiresNewPaymentMethod: false, _originalMessage: 'Original message', _stripeCode: 'card_declined', }; @@ -306,17 +225,17 @@ describe('Stripe Errors Utility', () => { expect(error).toBeInstanceOf(Error); expect(error.name).toBe('PaymentError'); - expect(error.code).toBe('bank_account_frozen'); - expect(error.ownerMessage).toBe("The renter's bank account is frozen."); - expect(error.renterMessage).toBe('Your bank account is frozen.'); - expect(error.requiresNewPaymentMethod).toBe(true); + expect(error.code).toBe('insufficient_funds'); + expect(error.ownerMessage).toBe("The renter's card has insufficient funds."); + expect(error.renterMessage).toBe('Your card has insufficient funds.'); + expect(error.requiresNewPaymentMethod).toBe(false); }); test('toJSON returns serializable object', () => { const parsedError = { - code: 'bank_account_closed', - ownerMessage: "The renter's bank account has been closed.", - renterMessage: 'Your bank account has been closed.', + code: 'expired_card', + ownerMessage: "The renter's card has expired.", + renterMessage: 'Your card has expired.', canOwnerRetry: false, requiresNewPaymentMethod: true, _originalMessage: 'Should not appear', @@ -326,7 +245,7 @@ describe('Stripe Errors Utility', () => { const error = new PaymentError(parsedError); const json = error.toJSON(); - expect(json).toHaveProperty('code', 'bank_account_closed'); + expect(json).toHaveProperty('code', 'expired_card'); expect(json).toHaveProperty('ownerMessage'); expect(json).toHaveProperty('renterMessage'); expect(json).not.toHaveProperty('_originalMessage'); diff --git a/backend/utils/stripeErrors.js b/backend/utils/stripeErrors.js index 16b70a5..cb5891f 100644 --- a/backend/utils/stripeErrors.js +++ b/backend/utils/stripeErrors.js @@ -5,6 +5,8 @@ * Provides structured error information for frontend handling. */ +const logger = require('./logger'); + const DECLINE_MESSAGES = { authentication_required: { ownerMessage: "The renter's card requires additional authentication.", @@ -218,50 +220,6 @@ const DECLINE_MESSAGES = { canOwnerRetry: false, requiresNewPaymentMethod: true, }, - - // ACH/Bank Account Declines - bank_account_closed: { - ownerMessage: "The renter's bank account has been closed.", - renterMessage: - "Your bank account has been closed. Please add a different payment method.", - canOwnerRetry: false, - requiresNewPaymentMethod: true, - }, - bank_account_frozen: { - ownerMessage: "The renter's bank account is frozen.", - renterMessage: - "Your bank account is frozen. Please contact your bank or use a different payment method.", - canOwnerRetry: false, - requiresNewPaymentMethod: true, - }, - bank_account_restricted: { - ownerMessage: "The renter's bank account has restrictions.", - renterMessage: - "Your bank account has restrictions. Please contact your bank or use a different payment method.", - canOwnerRetry: false, - requiresNewPaymentMethod: true, - }, - bank_account_unusable: { - ownerMessage: "The renter's bank account cannot be used.", - renterMessage: - "Your bank account cannot be used for payments. Please add a different payment method.", - canOwnerRetry: false, - requiresNewPaymentMethod: true, - }, - bank_account_invalid_details: { - ownerMessage: "The renter's bank account details are invalid.", - renterMessage: - "Your bank account details appear to be invalid. Please re-enter your bank information.", - canOwnerRetry: false, - requiresNewPaymentMethod: true, - }, - debit_not_authorized: { - ownerMessage: "The renter's bank account is not authorized for debits.", - renterMessage: - "Your bank account is not authorized for automatic debits. Please enable ACH payments or use a card.", - canOwnerRetry: false, - requiresNewPaymentMethod: true, - }, }; // Default error for unknown decline codes @@ -273,6 +231,37 @@ const DEFAULT_ERROR = { requiresNewPaymentMethod: true, }; +// Mapping for StripeInvalidRequestError codes +const INVALID_REQUEST_MESSAGES = { + resource_missing: { + ownerMessage: "The renter's payment method is no longer valid.", + renterMessage: + "Your payment method is no longer valid. Please add a new payment method.", + canOwnerRetry: false, + requiresNewPaymentMethod: true, + }, + payment_method_invalid: { + ownerMessage: "The renter's payment method is invalid.", + renterMessage: + "Your payment method is invalid. Please add a new payment method.", + canOwnerRetry: false, + requiresNewPaymentMethod: true, + }, + payment_intent_unexpected_state: { + ownerMessage: "This payment is in an unexpected state.", + renterMessage: "There was an issue with your payment. Please try again.", + canOwnerRetry: true, + requiresNewPaymentMethod: false, + }, + customer_deleted: { + ownerMessage: "The renter's payment profile has been deleted.", + renterMessage: + "Your payment profile needs to be set up again. Please add a new payment method.", + canOwnerRetry: false, + requiresNewPaymentMethod: true, + }, +}; + /** * Parse a Stripe error and return structured error information * @param {Error} error - The error object from Stripe @@ -284,6 +273,16 @@ function parseStripeError(error) { const declineCode = error.decline_code || error.code || "card_declined"; const errorInfo = DECLINE_MESSAGES[declineCode] || DEFAULT_ERROR; + // Log if we're falling back to default for an unknown decline code + if (!DECLINE_MESSAGES[declineCode]) { + logger.warn("[StripeErrors] Unknown decline code - please add mapping", { + declineCode, + errorCode: error.code, + errorType: error.type, + errorMessage: error.message, + }); + } + return { code: declineCode, ownerMessage: errorInfo.ownerMessage, @@ -298,8 +297,62 @@ function parseStripeError(error) { // Handle other Stripe error types if (error.type === "StripeInvalidRequestError") { + // First, check if there's a decline_code in the error (some bank errors include this) + if (error.decline_code && DECLINE_MESSAGES[error.decline_code]) { + const errorInfo = DECLINE_MESSAGES[error.decline_code]; + return { + code: error.decline_code, + ownerMessage: errorInfo.ownerMessage, + renterMessage: errorInfo.renterMessage, + canOwnerRetry: errorInfo.canOwnerRetry, + requiresNewPaymentMethod: errorInfo.requiresNewPaymentMethod, + _originalMessage: error.message, + _stripeCode: error.code, + }; + } + + // Check if error.code has a specific mapping + const errorCode = error.code; + if (errorCode && INVALID_REQUEST_MESSAGES[errorCode]) { + const errorInfo = INVALID_REQUEST_MESSAGES[errorCode]; + return { + code: errorCode, + ownerMessage: errorInfo.ownerMessage, + renterMessage: errorInfo.renterMessage, + canOwnerRetry: errorInfo.canOwnerRetry, + requiresNewPaymentMethod: errorInfo.requiresNewPaymentMethod, + _originalMessage: error.message, + _stripeCode: error.code, + }; + } + + // Also check DECLINE_MESSAGES for the error code (some codes appear in both contexts) + if (errorCode && DECLINE_MESSAGES[errorCode]) { + const errorInfo = DECLINE_MESSAGES[errorCode]; + return { + code: errorCode, + ownerMessage: errorInfo.ownerMessage, + renterMessage: errorInfo.renterMessage, + canOwnerRetry: errorInfo.canOwnerRetry, + requiresNewPaymentMethod: errorInfo.requiresNewPaymentMethod, + _originalMessage: error.message, + _stripeCode: error.code, + }; + } + + // Log unhandled StripeInvalidRequestError codes for future mapping + logger.warn( + "[StripeErrors] Unhandled StripeInvalidRequestError - please add mapping", + { + errorCode: error.code, + declineCode: error.decline_code, + errorMessage: error.message, + param: error.param, + } + ); + return { - code: "invalid_request", + code: errorCode || "invalid_request", ownerMessage: "There was a problem processing this payment.", renterMessage: "There was a problem with your payment method.", canOwnerRetry: false, @@ -337,6 +390,13 @@ function parseStripeError(error) { } // Default fallback for unknown errors + logger.warn("[StripeErrors] Unknown error type - please investigate", { + errorType: error.type, + errorCode: error.code, + declineCode: error.decline_code, + errorMessage: error.message, + }); + return { code: "unknown_error", ...DEFAULT_ERROR, diff --git a/frontend/src/pages/ItemDetail.tsx b/frontend/src/pages/ItemDetail.tsx index d23664a..6a96124 100644 --- a/frontend/src/pages/ItemDetail.tsx +++ b/frontend/src/pages/ItemDetail.tsx @@ -660,9 +660,15 @@ const ItemDetail: React.FC = () => { })()} {/* Rental Period Selection - Only show for non-owners */} - {!isOwner && item.isAvailable && !isAlreadyRenting && ( + {!isOwner && item.isAvailable && ( <>
+ {isAlreadyRenting && ( +
+ + You already have an active rental for this item +
+ )}
{dateValidationError && (
{ )} {/* Action Buttons */} - {!isOwner && item.isAvailable && !isAlreadyRenting && ( + {!isOwner && item.isAvailable && (
)} - {!isOwner && isAlreadyRenting && ( -
- -
- )}
@@ -906,9 +901,15 @@ const ItemDetail: React.FC = () => { })()} {/* Rental Period Selection - Only show for non-owners */} - {!isOwner && item.isAvailable && !isAlreadyRenting && ( + {!isOwner && item.isAvailable && ( <>
+ {isAlreadyRenting && ( +
+ + You already have an active rental for this item +
+ )}
{dateValidationError && (
{ )} {/* Action Buttons */} - {!isOwner && item.isAvailable && !isAlreadyRenting && ( + {!isOwner && item.isAvailable && (
)} - {!isOwner && isAlreadyRenting && ( -
- -
- )}
@@ -1120,30 +1110,20 @@ const ItemDetail: React.FC = () => { ); })()} - {isAlreadyRenting ? ( - - ) : ( - - )} + )} diff --git a/frontend/src/pages/Renting.tsx b/frontend/src/pages/Renting.tsx index 21e5233..c1d5370 100644 --- a/frontend/src/pages/Renting.tsx +++ b/frontend/src/pages/Renting.tsx @@ -388,8 +388,9 @@ const Renting: React.FC = () => {
- Payment issue: Please update your - payment method so the owner can approve your request. + Payment issue:{" "} + {rental.paymentFailedReason || + "Please update your payment method so the owner can approve your request."}