[Hotfix PROD] - duplicated payments #96

kkartunov · 2025-10-22T11:21:51Z

No description provided.

Remove rewards deps

… tx time is too long

…-on-payment When withdrawing, do not use transaction

github-actions · 2025-10-22T11:22:13Z

src/api/wallet/wallet.service.ts

  getWinningsTotalsByWinnerID(winnerId: string) {
    return this.prisma.$queryRaw<
-      { payment_type: 'PAYMENT' | 'REWARD'; total_owed: number }[]
+      { payment_type: 'PAYMENT'; total_owed: number }[]


[❗❗ correctness]
The removal of 'REWARD' from the payment_type union may impact the correctness of the query results if REWARD payments are still relevant to the business logic. Ensure that this change aligns with the intended functionality and that REWARD payments are handled appropriately elsewhere if needed.

github-actions · 2025-10-22T11:22:21Z

src/api/withdrawal/withdrawal.service.ts

-    tx: Prisma.TransactionClient,
  ) {
-    const winnings = await tx.$queryRaw<ReleasableWinningRow[]>`
+    const winnings = await this.prisma.$queryRaw<ReleasableWinningRow[]>`


[❗❗ correctness]
Switching from tx.$queryRaw to this.prisma.$queryRaw changes the transaction context. Ensure that this change does not affect transaction integrity or lead to race conditions, especially in concurrent environments.

github-actions · 2025-10-22T11:22:25Z

src/api/withdrawal/withdrawal.service.ts

    return totalAmount;
  }

  private async createDbPaymentRelease(


[❗❗ correctness]
The removal of the tx: Prisma.TransactionClient parameter suggests that the transaction context might no longer be passed to createDbPaymentRelease. Ensure that this change does not affect the transactional integrity of the operation, as it could lead to partial updates or data inconsistencies if the function was previously relying on a transaction context.

github-actions · 2025-10-22T11:22:27Z

src/api/withdrawal/withdrawal.service.ts

  ) {
    try {
-      const paymentRelease = await tx.payment_releases.create({
+      const paymentRelease = await this.prisma.payment_releases.create({


[❗❗ correctness]
Switching from tx.payment_releases.create to this.prisma.payment_releases.create changes the context from a transaction-scoped operation to a potentially non-transactional one. Ensure that this change does not inadvertently affect transactional integrity, especially in a hotfix context where duplicated payments are a concern.

github-actions · 2025-10-22T11:22:30Z

src/api/withdrawal/withdrawal.service.ts

  ) {
    try {
-      await tx.payment_releases.update({
+      await this.prisma.payment_releases.update({


[❗❗ correctness]
Switching from tx to this.prisma for database updates changes the transaction context. Ensure that this does not affect transaction integrity, especially if tx was part of a larger transaction scope.

github-actions · 2025-10-22T11:22:31Z

src/api/withdrawal/withdrawal.service.ts

-      this.logger.log(
-        `DB payment_release[${paymentRelease.payment_release_id}] updated successfully with trolley payment id: ${externalTxId}`,
-      );
+      if (data.externalTxId) {


[⚠️ maintainability]
The conditional logging based on data.externalTxId might lead to missing logs if externalTxId is not provided. Consider logging the update operation regardless of externalTxId presence for better traceability.

github-actions · 2025-10-22T11:22:34Z

src/api/withdrawal/withdrawal.service.ts

      throw new Error('Failed to fetch UserInfo for withdrawal!');
    }

+    if (userInfo.email.toLowerCase().indexOf('wipro.com') > -1) {


[⚠️ maintainability]
The check for the email domain wipro.com is hardcoded. Consider externalizing this configuration to allow for easier updates and maintenance, especially if more domains need to be added or changed in the future.

github-actions · 2025-10-22T11:22:34Z

src/api/withdrawal/withdrawal.service.ts

      throw new Error('Failed to fetch UserInfo for withdrawal!');
    }

+    if (userInfo.email.toLowerCase().indexOf('wipro.com') > -1) {


[💡 readability]
The use of indexOf for checking the presence of a substring can be replaced with includes for better readability and clarity.

github-actions · 2025-10-22T11:22:38Z

src/api/withdrawal/withdrawal.service.ts


-        const paymentBatch = await this.trolleyService.startBatchPayment(
-          `${userId}_${userHandle}`,
+      try {


[❗❗ correctness]
The try block starting here does not have a corresponding catch block to handle potential errors from this.paymentsService.updatePaymentProcessingState. This could lead to unhandled promise rejections if an error occurs. Consider adding a catch block to handle errors appropriately.

github-actions · 2025-10-22T11:22:38Z

src/api/withdrawal/withdrawal.service.ts

-          this.logger.error(errorMsg, error);
-          throw new Error(errorMsg);
-        }
+      try {


[❗❗ correctness]
The try block starting here does not have a corresponding catch block to handle potential errors from this.trolleyService.startProcessingPayment. This could lead to unhandled promise rejections if an error occurs. Consider adding a catch block to handle errors appropriately.

github-actions · 2025-10-22T11:22:38Z

src/api/withdrawal/withdrawal.service.ts

+        throw new Error(errorMsg);
+      }
+
+      try {


[❗❗ correctness]
The try block starting here does not have a corresponding catch block to handle potential errors from this.tcChallengesService.updateLegacyPayments. This could lead to unhandled promise rejections if an error occurs. Consider adding a catch block to handle errors appropriately.

github-actions · 2025-10-22T11:22:43Z

src/shared/payments/payments.service.ts

-      const prismaClient = transaction || this.prisma;
-
-      const r = await prismaClient.payment.updateMany({
+      const r = await this.prisma.payment.updateMany({


[❗❗ correctness]
Removing the ability to pass a transaction object might impact transactional integrity if this method is intended to be used within a transaction context. Consider whether this change could lead to unintended side effects, such as partial updates if the method is called within a transaction block elsewhere.

…-on-payment Remove payment if error is thrown when db data is updated

github-actions · 2025-10-22T11:28:54Z

src/api/withdrawal/withdrawal.service.ts

+        );
+      } catch (e) {
+        this.logger.error(
+          `Failed to update payment processing state: ${e?.message} for winnings '${winningsIds.join(',')}`,


[⚠️ correctness]
Using optional chaining (e?.message) is a good practice to avoid runtime errors when e is undefined or null. However, ensure that e is always an object or undefined in this context to prevent any unexpected behavior. Consider adding a fallback message for cases where e might not be an object.

github-actions · 2025-10-22T11:28:57Z

src/api/withdrawal/withdrawal.service.ts

+          status: 'FAILED',
+        });
+
+        await this.trolleyService.removePayment(


[❗❗ correctness]
Consider handling errors that might occur during removePayment. If removePayment fails, it could leave the system in an inconsistent state. Ensure that any errors are logged or handled appropriately to maintain system integrity.

…-on-payment add missing method

github-actions · 2025-10-22T12:03:07Z

src/shared/global/trolley.service.ts


+  async removePayment(paymentId: string, batchId: string) {
+    try {
+      await this.client.payment.remove(paymentId, batchId);


[⚠️ maintainability]
Consider handling specific error types separately to provide more granular error handling and potentially recover from certain errors. This can improve the robustness of the error handling logic.

github-actions · 2025-10-22T12:03:07Z

src/shared/global/trolley.service.ts

+      this.logger.debug(`Removed trolley payment with id ${paymentId}`);
+    } catch (error) {
+      this.logger.error(
+        `Failed to remove trolley payment: '${error.message}'!`,


[💡 readability]
Logging the error message alone may not provide sufficient context for debugging. Consider including additional error properties, such as error.code or error.stack, if available, to enhance the log output.

…-on-payment_lock-otp lock otp db row

github-actions · 2025-10-22T12:15:05Z

src/api/withdrawal/withdrawal.service.ts

+          return { error: otpResponse };
+        }
+      } catch (error) {
+        if (error.code === 'P2010' && error.meta?.code === '55P03') {


[⚠️ maintainability]
The error handling for P2010 with meta.code 55P03 is specific and may not cover other potential error scenarios. Consider logging or handling other error codes to ensure robustness.

github-actions · 2025-10-22T12:15:05Z

src/api/withdrawal/withdrawal.service.ts

+            error,
+          );
+
+          throw new Error(


[⚠️ design]
Throwing a generic Error with a message might not provide enough context for upstream error handling. Consider using a custom error type to encapsulate additional context or metadata.

github-actions · 2025-10-22T12:15:09Z

src/shared/global/otp.service.ts

-      };
-    }
+    return await this.prisma.$transaction(async (tx) => {
+      const records = await tx.$queryRaw<otp>`


[❗❗ security]
Using tx.$queryRaw with string interpolation for SQL queries can lead to SQL injection vulnerabilities. Consider using parameterized queries to ensure safety.

github-actions · 2025-10-22T12:15:09Z

src/shared/global/otp.service.ts

+        WHERE otp_hash=${hashOtp(otpCode)}
+        ORDER BY expiration_time DESC
+        LIMIT 1
+        FOR UPDATE NOWAIT;


[⚠️ correctness]
The use of FOR UPDATE NOWAIT in the query may lead to transaction failures if the row is locked by another transaction. Ensure that the application logic can handle such cases gracefully.

vas3a and others added 4 commits October 22, 2025 13:22

Remove any reference to rewards

59c1e09

Merge pull request #94 from topcoder-platform/remove-rewards-deps

8fb4c09

Remove rewards deps

When withdrawing, do not use transaction, as this can cause issues if…

94461a5

… tx time is too long

Merge pull request #95 from topcoder-platform/hotfix-drop-transaction…

bce5825

…-on-payment When withdrawing, do not use transaction

github-actions bot reviewed Oct 22, 2025

View reviewed changes

vas3a and others added 2 commits October 22, 2025 14:24

Remove payment if error is thrown when db data is updated

42749c6

Merge pull request #97 from topcoder-platform/hotfix-drop-transaction…

b41c474

…-on-payment Remove payment if error is thrown when db data is updated

github-actions bot reviewed Oct 22, 2025

View reviewed changes

vas3a and others added 3 commits October 22, 2025 15:00

lock otp db row

bf2fd8d

add missing method

95e072d

Merge pull request #98 from topcoder-platform/hotfix-drop-transaction…

ac81770

…-on-payment add missing method

github-actions bot reviewed Oct 22, 2025

View reviewed changes

Merge pull request #99 from topcoder-platform/hotfix-drop-transaction…

120fbc5

…-on-payment_lock-otp lock otp db row

github-actions bot reviewed Oct 22, 2025

View reviewed changes

kkartunov merged commit 3f297e3 into master Oct 22, 2025
2 checks passed

Uh oh!

[Hotfix PROD] - duplicated payments #96

[Hotfix PROD] - duplicated payments #96

Uh oh!

Conversation

kkartunov commented Oct 22, 2025

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants