Handle async stripe subscription retry #11187
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| if (order.Subscription.chargeNumber !== null) { | ||
| order.Subscription.chargeNumber += 1; | ||
| } | ||
|
|
||
| order.status = status.ACTIVE; | ||
| } | ||
|
|
||
| if (orderProcessedStatus === 'success' || orderProcessedStatus === 'processing') { | ||
| // TODO: we should consolidate on error and remove latestError | ||
| order.data = omit(order.data, ['error', 'latestError']); | ||
| } |
There was a problem hiding this comment.
Bug: The status.PROCESSING constant is undefined, causing exclusion logic to fail and order.status not to be set during processing.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The query at line 34 attempts to exclude orders with status.PROCESSING, but this constant is undefined. Consequently, when an order's payment is in a processing state (lines 152-165), its order.status is not explicitly updated to a processing status. This allows the order to be repeatedly fetched by subsequent cron runs, as the intended exclusion based on a processing status is ineffective.
💡 Suggested Fix
Define the status.PROCESSING constant. Ensure order.status is explicitly set to status.PROCESSING when payment is in a processing state.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: server/lib/recurring-contributions.js#L153-L163
Potential issue: The query at line 34 attempts to exclude orders with
`status.PROCESSING`, but this constant is undefined. Consequently, when an order's
payment is in a processing state (lines 152-165), its `order.status` is not explicitly
updated to a processing status. This allows the order to be repeatedly fetched by
subsequent cron runs, as the intended exclusion based on a processing status is
ineffective.
Did we get this right? 👍 / 👎 to inform future reviews.
| interval, | ||
| monthlyInterval: interval === 'month', | ||
| firstPayment: true, | ||
| firstPayment, |
There was a problem hiding this comment.
This default value has been removed and passed as an optional argument, yet I don't see all the references to sendEmailNotifications being updated, only one of them. Aren't we going to miss it with other payment methods, when called from executeOrder?
There was a problem hiding this comment.
Thanks, I kept the existing default now.
Resolve opencollective/opencollective#8251