mirror of
https://github.com/xtr-dev/payload-billing.git
synced 2025-12-10 02:43:24 +00:00
Enhance webhook detection with explicit context tracking and database optimization
- Add database index on version field for optimistic locking performance - Implement explicit webhook context tracking with symbols to avoid conflicts - Replace fragile webhook detection logic with robust context-based approach - Add request metadata support for enhanced debugging and audit trails - Simplify version management in payment collection hooks - Fix TypeScript compilation errors and improve type safety 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,7 @@ import type { AccessArgs, CollectionBeforeChangeHook, CollectionConfig, Collecti
|
|||||||
import type { BillingPluginConfig} from '@/plugin/config';
|
import type { BillingPluginConfig} from '@/plugin/config';
|
||||||
import { defaults } from '@/plugin/config'
|
import { defaults } from '@/plugin/config'
|
||||||
import { extractSlug, toPayloadId } from '@/plugin/utils'
|
import { extractSlug, toPayloadId } from '@/plugin/utils'
|
||||||
|
import { isWebhookRequest } from '@/providers/context'
|
||||||
import { Payment } from '@/plugin/types/payments'
|
import { Payment } from '@/plugin/types/payments'
|
||||||
import { initProviderPayment } from '@/collections/hooks'
|
import { initProviderPayment } from '@/collections/hooks'
|
||||||
|
|
||||||
@@ -114,6 +115,7 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col
|
|||||||
},
|
},
|
||||||
defaultValue: 1,
|
defaultValue: 1,
|
||||||
required: true,
|
required: true,
|
||||||
|
index: true, // Index for faster optimistic lock queries
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
if (overrides?.fields) {
|
if (overrides?.fields) {
|
||||||
@@ -136,7 +138,7 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col
|
|||||||
fields,
|
fields,
|
||||||
hooks: {
|
hooks: {
|
||||||
beforeChange: [
|
beforeChange: [
|
||||||
async ({ data, operation, req }) => {
|
async ({ data, operation, req, originalDoc }) => {
|
||||||
if (operation === 'create') {
|
if (operation === 'create') {
|
||||||
// Initialize version for new payments
|
// Initialize version for new payments
|
||||||
data.version = 1
|
data.version = 1
|
||||||
@@ -158,25 +160,11 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col
|
|||||||
} else if (operation === 'update') {
|
} else if (operation === 'update') {
|
||||||
// Handle version incrementing for manual updates
|
// Handle version incrementing for manual updates
|
||||||
// Webhook updates from providers should already set the version via optimistic locking
|
// Webhook updates from providers should already set the version via optimistic locking
|
||||||
if (!data.version && req.id) {
|
if (!data.version && originalDoc?.id) {
|
||||||
// Check if this is a webhook update by looking for webhook-specific fields
|
// Check if this is a webhook update using explicit context tracking
|
||||||
const isWebhookUpdate = data.providerData &&
|
if (!isWebhookRequest(req)) {
|
||||||
(data.providerData.webhookProcessedAt ||
|
|
||||||
(typeof data.providerData === 'object' && 'webhookProcessedAt' in data.providerData))
|
|
||||||
|
|
||||||
if (!isWebhookUpdate) {
|
|
||||||
// This is a manual admin update, safely increment version
|
// This is a manual admin update, safely increment version
|
||||||
try {
|
data.version = (originalDoc.version || 1) + 1
|
||||||
const currentDoc = await req.payload.findByID({
|
|
||||||
collection: extractSlug(pluginConfig.collections?.payments || defaults.paymentsCollection),
|
|
||||||
id: toPayloadId(req.id)
|
|
||||||
})
|
|
||||||
data.version = (currentDoc?.version || 1) + 1
|
|
||||||
} catch (error) {
|
|
||||||
// If we can't find the current document, start with version 1
|
|
||||||
console.warn(`[Payment Hook] Could not fetch current version for payment ${req.id}, defaulting to version 1:`, error)
|
|
||||||
data.version = 1
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
// If it's a webhook update without a version, let it proceed (optimistic locking already handled it)
|
// If it's a webhook update without a version, let it proceed (optimistic locking already handled it)
|
||||||
}
|
}
|
||||||
|
|||||||
50
src/providers/context.ts
Normal file
50
src/providers/context.ts
Normal file
@@ -0,0 +1,50 @@
|
|||||||
|
/**
|
||||||
|
* Request context utilities for tracking webhook vs manual operations
|
||||||
|
*/
|
||||||
|
|
||||||
|
// Symbol for storing webhook context in the request object
|
||||||
|
const WEBHOOK_CONTEXT_SYMBOL = Symbol('billingWebhookContext')
|
||||||
|
|
||||||
|
export interface WebhookContext {
|
||||||
|
isWebhookUpdate: boolean
|
||||||
|
provider?: string
|
||||||
|
webhookType?: string
|
||||||
|
timestamp: string
|
||||||
|
metadata?: Record<string, any>
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Mark a request as coming from a webhook
|
||||||
|
*/
|
||||||
|
export function markRequestAsWebhook(
|
||||||
|
req: any,
|
||||||
|
provider: string,
|
||||||
|
webhookType: string = 'payment_update',
|
||||||
|
metadata?: Record<string, any>
|
||||||
|
): void {
|
||||||
|
const context: WebhookContext = {
|
||||||
|
isWebhookUpdate: true,
|
||||||
|
provider,
|
||||||
|
webhookType,
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
metadata
|
||||||
|
}
|
||||||
|
|
||||||
|
// Store context in request object using symbol to avoid conflicts
|
||||||
|
req[WEBHOOK_CONTEXT_SYMBOL] = context
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if a request is from a webhook
|
||||||
|
*/
|
||||||
|
export function isWebhookRequest(req: any): boolean {
|
||||||
|
const context = req[WEBHOOK_CONTEXT_SYMBOL] as WebhookContext | undefined
|
||||||
|
return context?.isWebhookUpdate === true
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get webhook context from request
|
||||||
|
*/
|
||||||
|
export function getWebhookContext(req: any): WebhookContext | null {
|
||||||
|
return req[WEBHOOK_CONTEXT_SYMBOL] as WebhookContext || null
|
||||||
|
}
|
||||||
@@ -6,7 +6,7 @@ import type { createMollieClient, MollieClient } from '@mollie/api-client'
|
|||||||
import {
|
import {
|
||||||
webhookResponses,
|
webhookResponses,
|
||||||
findPaymentByProviderId,
|
findPaymentByProviderId,
|
||||||
updatePaymentStatus,
|
updatePaymentFromWebhook,
|
||||||
updateInvoiceOnPaymentSuccess,
|
updateInvoiceOnPaymentSuccess,
|
||||||
handleWebhookError,
|
handleWebhookError,
|
||||||
validateProductionUrl
|
validateProductionUrl
|
||||||
@@ -83,13 +83,15 @@ export const mollieProvider = (mollieConfig: MollieProviderConfig & {
|
|||||||
// Map Mollie status to our status using proper type-safe mapping
|
// Map Mollie status to our status using proper type-safe mapping
|
||||||
const status = mapMollieStatusToPaymentStatus(molliePayment.status)
|
const status = mapMollieStatusToPaymentStatus(molliePayment.status)
|
||||||
|
|
||||||
// Update the payment status and provider data
|
// Update the payment status and provider data with webhook context
|
||||||
const updateSuccess = await updatePaymentStatus(
|
const updateSuccess = await updatePaymentFromWebhook(
|
||||||
payload,
|
payload,
|
||||||
payment.id,
|
payment.id,
|
||||||
status,
|
status,
|
||||||
molliePayment.toPlainObject(),
|
molliePayment.toPlainObject(),
|
||||||
pluginConfig
|
pluginConfig,
|
||||||
|
'mollie',
|
||||||
|
`payment.${molliePayment.status}`
|
||||||
)
|
)
|
||||||
|
|
||||||
// If payment is successful and update succeeded, update the invoice
|
// If payment is successful and update succeeded, update the invoice
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import type Stripe from 'stripe'
|
|||||||
import {
|
import {
|
||||||
webhookResponses,
|
webhookResponses,
|
||||||
findPaymentByProviderId,
|
findPaymentByProviderId,
|
||||||
updatePaymentStatus,
|
updatePaymentFromWebhook,
|
||||||
updateInvoiceOnPaymentSuccess,
|
updateInvoiceOnPaymentSuccess,
|
||||||
handleWebhookError,
|
handleWebhookError,
|
||||||
logWebhookEvent
|
logWebhookEvent
|
||||||
@@ -74,7 +74,7 @@ export const stripeProvider = (stripeConfig: StripeProviderConfig) => {
|
|||||||
// Verify webhook signature and construct event
|
// Verify webhook signature and construct event
|
||||||
let event: Stripe.Event
|
let event: Stripe.Event
|
||||||
try {
|
try {
|
||||||
event = stripe.webhooks.constructEvent(body, signature, stripeConfig.webhookSecret)
|
event = stripe.webhooks.constructEvent(body, signature, stripeConfig.webhookSecret!)
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
return handleWebhookError('Stripe', err, 'Signature verification failed')
|
return handleWebhookError('Stripe', err, 'Signature verification failed')
|
||||||
}
|
}
|
||||||
@@ -117,12 +117,14 @@ export const stripeProvider = (stripeConfig: StripeProviderConfig) => {
|
|||||||
timestamp: new Date().toISOString(),
|
timestamp: new Date().toISOString(),
|
||||||
provider: 'stripe'
|
provider: 'stripe'
|
||||||
}
|
}
|
||||||
const updateSuccess = await updatePaymentStatus(
|
const updateSuccess = await updatePaymentFromWebhook(
|
||||||
payload,
|
payload,
|
||||||
payment.id,
|
payment.id,
|
||||||
status,
|
status,
|
||||||
providerData,
|
providerData,
|
||||||
pluginConfig
|
pluginConfig,
|
||||||
|
'stripe',
|
||||||
|
event.type
|
||||||
)
|
)
|
||||||
|
|
||||||
// If payment is successful and update succeeded, update the invoice
|
// If payment is successful and update succeeded, update the invoice
|
||||||
@@ -163,12 +165,14 @@ export const stripeProvider = (stripeConfig: StripeProviderConfig) => {
|
|||||||
timestamp: new Date().toISOString(),
|
timestamp: new Date().toISOString(),
|
||||||
provider: 'stripe'
|
provider: 'stripe'
|
||||||
}
|
}
|
||||||
const updateSuccess = await updatePaymentStatus(
|
const updateSuccess = await updatePaymentFromWebhook(
|
||||||
payload,
|
payload,
|
||||||
payment.id,
|
payment.id,
|
||||||
isFullyRefunded ? 'refunded' : 'partially_refunded',
|
isFullyRefunded ? 'refunded' : 'partially_refunded',
|
||||||
providerData,
|
providerData,
|
||||||
pluginConfig
|
pluginConfig,
|
||||||
|
'stripe',
|
||||||
|
event.type
|
||||||
)
|
)
|
||||||
|
|
||||||
if (!updateSuccess) {
|
if (!updateSuccess) {
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import type { BillingPluginConfig } from '@/plugin/config'
|
|||||||
import type { ProviderData } from './types'
|
import type { ProviderData } from './types'
|
||||||
import { defaults } from '@/plugin/config'
|
import { defaults } from '@/plugin/config'
|
||||||
import { extractSlug, toPayloadId } from '@/plugin/utils'
|
import { extractSlug, toPayloadId } from '@/plugin/utils'
|
||||||
|
import { markRequestAsWebhook } from './context'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Common webhook response utilities
|
* Common webhook response utilities
|
||||||
@@ -43,6 +44,29 @@ export async function findPaymentByProviderId(
|
|||||||
return payments.docs.length > 0 ? payments.docs[0] as Payment : null
|
return payments.docs.length > 0 ? payments.docs[0] as Payment : null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Update payment status from webhook with proper context tracking
|
||||||
|
*/
|
||||||
|
export async function updatePaymentFromWebhook(
|
||||||
|
payload: Payload,
|
||||||
|
paymentId: string | number,
|
||||||
|
status: Payment['status'],
|
||||||
|
providerData: ProviderData<any>,
|
||||||
|
pluginConfig: BillingPluginConfig,
|
||||||
|
provider: string,
|
||||||
|
eventType?: string
|
||||||
|
): Promise<boolean> {
|
||||||
|
// Mark the request context as webhook before updating with metadata
|
||||||
|
markRequestAsWebhook((payload as any).req, provider, 'payment_status_update', {
|
||||||
|
paymentId: paymentId.toString(),
|
||||||
|
newStatus: status,
|
||||||
|
eventType,
|
||||||
|
timestamp: new Date().toISOString()
|
||||||
|
})
|
||||||
|
|
||||||
|
return updatePaymentStatus(payload, paymentId, status, providerData, pluginConfig)
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Update payment status and provider data with atomic optimistic locking
|
* Update payment status and provider data with atomic optimistic locking
|
||||||
*/
|
*/
|
||||||
@@ -65,13 +89,10 @@ export async function updatePaymentStatus(
|
|||||||
const nextVersion = (currentPayment.version || 1) + 1
|
const nextVersion = (currentPayment.version || 1) + 1
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Use updateMany for atomic version-based optimistic locking
|
// Use update with version check for atomic optimistic locking
|
||||||
const result = await payload.updateMany({
|
const updatedPayment = await payload.update({
|
||||||
collection: paymentsCollection,
|
collection: paymentsCollection,
|
||||||
where: {
|
id: toPayloadId(paymentId),
|
||||||
id: { equals: toPayloadId(paymentId) },
|
|
||||||
version: { equals: currentPayment.version || 1 }
|
|
||||||
},
|
|
||||||
data: {
|
data: {
|
||||||
status,
|
status,
|
||||||
version: nextVersion,
|
version: nextVersion,
|
||||||
@@ -80,17 +101,23 @@ export async function updatePaymentStatus(
|
|||||||
webhookProcessedAt: now,
|
webhookProcessedAt: now,
|
||||||
previousStatus: currentPayment.status
|
previousStatus: currentPayment.status
|
||||||
}
|
}
|
||||||
|
},
|
||||||
|
where: {
|
||||||
|
version: { equals: currentPayment.version || 1 }
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
// Check if the update was successful (affected documents > 0)
|
// If we get here without error, the update succeeded
|
||||||
if (result.docs && result.docs.length > 0) {
|
return true
|
||||||
return true
|
} catch (error: any) {
|
||||||
} else {
|
// Check if this is a version mismatch (no documents found to update)
|
||||||
|
if (error?.message?.includes('No documents found') ||
|
||||||
|
error?.name === 'ValidationError' ||
|
||||||
|
error?.status === 404) {
|
||||||
console.warn(`[Payment Update] Optimistic lock failed for payment ${paymentId} - version mismatch (expected: ${currentPayment.version}, may have been updated by another process)`)
|
console.warn(`[Payment Update] Optimistic lock failed for payment ${paymentId} - version mismatch (expected: ${currentPayment.version}, may have been updated by another process)`)
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
} catch (error) {
|
|
||||||
console.error(`[Payment Update] Failed to update payment ${paymentId}:`, error)
|
console.error(`[Payment Update] Failed to update payment ${paymentId}:`, error)
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user