Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(adapter-mssql): prevent connection pool exhaustion and EREQINPROG errors by serializing commit/rollback#28825

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.

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

Already on GitHub?Sign in to your account

Open
devanshuprakash wants to merge5 commits intoprisma:main
base:main
Choose a base branch
Loading
fromdevanshuprakash:fix/mssql-connection-pool-exhaustion

Conversation

@devanshuprakash
Copy link

@devanshuprakashdevanshuprakash commentedDec 3, 2025•
edited by coderabbitaibot
Loading

This PR fixes a critical issue in@prisma/adapter-mssql whereEREQINPROG errors and connection pool exhaustion could occur under load.

Root Cause

The issue was caused by a race condition inside theMssqlTransaction class:

  • Thecommit androllback methods did not acquire the mutex that serializes access to the underlying MSSQL connection.
  • As a result,commit orrollback could execute while a query was still in progress (e.g., a slow query).
  • The MSSQL driver then threw:

EREQINPROG: Can't rollback transaction. There is a request in progress.

  • In some cases, this left the connection in a bad state, eventually exhausting the connection pool.

Fix

This PR ensures:

  • commit acquires the mutex before executing.
  • rollback acquires the mutex before executing.

This guarantees that these methods wait for any active queries to finish, removing the race condition.

Changes

packages/adapter-mssql/src/mssql.ts

  • UpdatedMssqlTransaction.commit to acquire the mutex.
  • UpdatedMssqlTransaction.rollback to acquire the mutex.
  • ExportedPrismaMssqlAdapter to enable unit testing.

packages/adapter-mssql/src/transaction.test.ts

  • Added a regression test that reproduces the issue by:
    • Simulating a slow query,
    • Triggering a rollback while the query is active.
  • Verified that the fix prevents theEREQINPROG error and allows the rollback to complete successfully after the query finishes.

Verification

Tested via:

pnpm test packages/adapter-mssql/src/transaction.test.ts

Output:

✓ src/transaction.test.ts (1 test) 103ms
✓ PrismaMssqlAdapter reproduction > should reproduce EREQINPROG when rollback is called during active query 103ms

Fixes#28812

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved MSSQL transaction handling with enhanced commit and rollback operations for better reliability.

  • Tests

    • Added transaction scenario testing to verify rollback behavior during concurrent operations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitaibot commentedDec 3, 2025•
edited
Loading

Walkthrough

Addresses a race condition in MSSQL adapter transaction handling by protecting commit and rollback operations with mutex locks to prevent concurrent access, and adds a test validating rollback waits for in-progress queries to complete.

Changes

Cohort / File(s)Change Summary
Transaction synchronization
packages/adapter-mssql/src/mssql.ts		Wrapped commit and rollback operations with mutex acquisition/release to prevent concurrent access; exported PrismaMssqlAdapter class for public use
Transaction tests
packages/adapter-mssql/src/transaction.test.ts		New test file validating rollback behavior during in-flight queries and EREQINPROG error handling; verifies rollback waits for in-progress queries to complete

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe PR title accurately describes the primary fix: serializing commit/rollback operations to prevent EREQINPROG errors and connection pool exhaustion.
Linked Issues check✅ PassedThe PR addresses all key objectives from issue#28812: implements mutex serialization for commit/rollback, exports PrismaMssqlAdapter for testing, and adds regression test verifying proper handling of in-progress queries.
Out of Scope Changes check✅ PassedAll changes directly support the fix for issue#28812: mutex wrapping of commit/rollback, exporting adapter class for testing, and transaction rollback regression test. No unrelated modifications detected.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between708f39f and5c110e8.

📒 Files selected for processing (2)
  • packages/adapter-mssql/src/mssql.ts (2 hunks)
  • packages/adapter-mssql/src/transaction.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript for new code in the Prisma monorepo

Files:

  • packages/adapter-mssql/src/transaction.test.ts
  • packages/adapter-mssql/src/mssql.ts
**/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.test.ts: Place test files alongside source files with.test.ts naming convention, which are excluded from build output via esbuild config
Inline snapshots should be kept concise unless the exact message matters

Files:

  • packages/adapter-mssql/src/transaction.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-03T09:46:36.091Z
Learnt from: CRRepo: prisma/prisma PR: 0File: AGENTS.md:0-0Timestamp: 2025-12-03T09:46:36.091ZLearning: Applies to packages/migrate/src/__tests__/**/*.test.ts : Use `ctx.setDatasource()` test helper to override config.datasource for connection-specific test scenarios

Applied to files:

  • packages/adapter-mssql/src/transaction.test.ts
📚 Learning: 2025-12-03T09:46:36.091Z
Learnt from: CRRepo: prisma/prisma PR: 0File: AGENTS.md:0-0Timestamp: 2025-12-03T09:46:36.091ZLearning: Applies to packages/client-generator-ts/src/TSClient/file-generators/PrismaNamespaceFile.ts : In PrismaClient constructor options, update generated types in `packages/client-generator-ts/src/TSClient/file-generators/PrismaNamespaceFile.ts` (`buildClientOptions` function)

Applied to files:

  • packages/adapter-mssql/src/mssql.ts
📚 Learning: 2025-12-03T09:46:36.091Z
Learnt from: CRRepo: prisma/prisma PR: 0File: AGENTS.md:0-0Timestamp: 2025-12-03T09:46:36.091ZLearning: Applies to packages/client-generator-js/src/TSClient/PrismaClient.ts : In PrismaClient constructor options, update generated types in `packages/client-generator-js/src/TSClient/PrismaClient.ts` (`buildClientOptions` method)

Applied to files:

  • packages/adapter-mssql/src/mssql.ts
📚 Learning: 2025-12-03T09:46:36.091Z
Learnt from: CRRepo: prisma/prisma PR: 0File: AGENTS.md:0-0Timestamp: 2025-12-03T09:46:36.091ZLearning: Applies to packages/client/src/runtime/core/engines/common/Engine.ts : In PrismaClient constructor options, update engine config in `EngineConfig` interface in `packages/client/src/runtime/core/engines/common/Engine.ts`

Applied to files:

  • packages/adapter-mssql/src/mssql.ts
🧬 Code graph analysis (1)
packages/adapter-mssql/src/transaction.test.ts (2)
packages/adapter-mssql/src/mssql.ts (1)
  • PrismaMssqlAdapter (128-177)
packages/client-engine-runtime/src/transaction-manager/transaction-manager.ts (1)
  • tx (239-295)
🔇 Additional comments (5)
packages/adapter-mssql/src/mssql.ts (3)

95-100:LGTM: Mutex correctly serializes commit with queries.

The mutex acquisition ensurescommit() cannot run concurrently with an in-progress query, preventing the race condition. Thefinally block guarantees mutex release even if commit throws.

106-118:LGTM: Mutex correctly serializes rollback while preserving EABORT handling.

The mutex acquisition preventsrollback() from running during an active query, addressing the EREQINPROG error. The preserved EABORT handling correctly ignores errors when the transaction was already aborted, and thefinally block ensures mutex release in all cases.

128-128:LGTM: Export enables testing.

ExportingPrismaMssqlAdapter expands the public API surface but is necessary for the regression test added in this PR.

packages/adapter-mssql/src/transaction.test.ts (2)

1-27:LGTM: Mock setup provides minimal viable implementation.

The mockedmssql module provides the necessary structure for the test. The empty object returned bytransaction() is appropriately overridden in the test with a complete mock implementation.

29-96:LGTM: Test comprehensively validates the race condition fix.

The test accurately simulates the original issue:

  1. Starts a slow query that setsrequestActive = true
  2. Attempts rollback while the query is active
  3. The mutex (from the fix) causes rollback to wait for the query to complete
  4. When rollback executes,requestActive = false, so EREQINPROG is not thrown

The test name, type assertions, and mock verifications properly address all previous review feedback.

Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment@coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweend81b4b6 and4926bf0.

📒 Files selected for processing (3)
  • packages/adapter-mssql/src/mssql.ts (2 hunks)
  • packages/adapter-mssql/src/transaction.test.ts (1 hunks)
  • packages/cli/src/Init.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/adapter-mssql/src/transaction.test.ts (2)
packages/adapter-mssql/src/mssql.ts (1)
  • PrismaMssqlAdapter (128-177)
packages/client-engine-runtime/src/transaction-manager/transaction-manager.ts (1)
  • tx (239-295)
packages/adapter-mssql/src/mssql.ts (3)
packages/client/tests/e2e/_utils/run.ts (1)
  • release (248-254)
packages/driver-adapter-utils/src/types.ts (1)
  • SqlDriverAdapter (248-268)
packages/driver-adapter-utils/src/index.ts (1)
  • SqlDriverAdapter (28-28)
🔇 Additional comments (3)
packages/cli/src/Init.ts (1)

116-139:Lazy-loading dev server modules only when needed looks correct

Guarding the dynamicimport('@prisma/dev') /import('@prisma/dev/internal/state') behindurl === undefined preserves existing behavior for all code paths while avoiding unnecessary module load when a URL is already provided. ThedefaultEnv callers always pass a defined URL except for the “local Prisma Postgres” default case, so the change is behaviorally equivalent and lower overhead.

packages/adapter-mssql/src/mssql.ts (2)

92-119:Serializing commit/rollback with the same mutex as performIO aligns with the intended fix

Usingthis.#mutex.acquire() in bothcommit androllback, withrelease() in afinally block, correctly ensures that commits/rollbacks cannot run while a transactional request is in progress, and that queries cannot start while a close operation is running. This should prevent the EREQINPROG “request in progress” race while still preserving the existing EABORT handling inrollback.

128-210:ExportingPrismaMssqlAdapter is consistent with its factory and useful for tests

MakingPrismaMssqlAdapter an exported class matches its use as the concrete return type ofPrismaMssqlAdapterFactory.connect() and enables direct instantiation in tests and advanced consumers. Just be aware this effectively promotes it to part of the package’s public API surface going forward.

Copy link
Member

@aqrlnaqrln left a comment

Choose a reason for hiding this comment

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

Thanks so much! The change in the logic itself looks good to me, but there are some issues in the test and unrelated changes that need to be moved out of this PR.

devanshuprakash reacted with laugh emoji
@aqrlnaqrln added this to the7.2.0 milestoneDec 3, 2025
@devanshuprakash
Copy link
Author

I've addressed your feedback:

  1. Renamed the test suite toPrismaMssqlAdapter transaction regression to better reflect its purpose.
  2. Removed theany usage in the test and replaced it with a proper type assertion (Error & { code: string }).
    Please review@aqrln

Copy link
Contributor

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
packages/adapter-mssql/src/transaction.test.ts (3)

29-29:Test suite name should be more specific.

The suite name "transaction regression" doesn't clearly indicate what race condition is being guarded against. Consider a more descriptive name like:

-describe('PrismaMssqlAdapter transaction regression', () => {+describe('PrismaMssqlAdapter: rollback during active query (EREQINPROG prevention)', () => {

30-30:Test name contradicts the expected behavior.

The test name says "should reproduce EREQINPROG" but line 87 expects rollback to resolve without error. With the mutex fix in place, the test verifies that EREQINPROG doesnot occur. Consider renaming to reflect the guarded regression:

-  it('should reproduce EREQINPROG when rollback is called during active query', async () => {+  it('does not throw EREQINPROG when rollback is called during an active query', async () => {

62-73:Avoid usingas any type assertion.

Theas any type assertion on line 73 bypasses type safety. Consider defining a proper type or interface for the mocked transaction object:

typeMockTransaction={on:()=>voidbegin:()=>Promise<void>commit:()=>Promise<void>rollback:ReturnType<typeofvi.fn>request:()=>{input:()=>voidquery:ReturnType<typeofvi.fn>arrayRowMode:boolean}}pool.transaction=()=>({on:()=>{},begin:async()=>{},commit:async()=>{},rollback:rollbackMock,request:()=>({input:()=>{},query:queryMock,arrayRowMode:false,}),})asMockTransaction
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between4926bf0 and76d3a21.

📒 Files selected for processing (1)
  • packages/adapter-mssql/src/transaction.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript for new code in the Prisma monorepo

Files:

  • packages/adapter-mssql/src/transaction.test.ts
**/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.test.ts: Place test files alongside source files with.test.ts naming convention, which are excluded from build output via esbuild config
Inline snapshots should be kept concise unless the exact message matters

Files:

  • packages/adapter-mssql/src/transaction.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-03T09:46:36.091Z
Learnt from: CRRepo: prisma/prisma PR: 0File: AGENTS.md:0-0Timestamp: 2025-12-03T09:46:36.091ZLearning: Applies to packages/migrate/src/__tests__/**/*.test.ts : Use `ctx.setDatasource()` test helper to override config.datasource for connection-specific test scenarios

Applied to files:

  • packages/adapter-mssql/src/transaction.test.ts
🧬 Code graph analysis (1)
packages/adapter-mssql/src/transaction.test.ts (2)
packages/adapter-mssql/src/mssql.ts (1)
  • PrismaMssqlAdapter (128-177)
packages/client-engine-runtime/src/transaction-manager/transaction-manager.ts (1)
  • tx (239-295)
🔇 Additional comments (2)
packages/adapter-mssql/src/transaction.test.ts (2)

1-4:LGTM!

Imports are appropriate for the test setup. The newly exportedPrismaMssqlAdapter class enables this unit test.

6-27:LGTM!

The mocking strategy appropriately isolates the adapter behavior for testing. The minimal implementations and override pattern are standard for unit tests.

@devanshuprakash
Copy link
Author

@aqrln
Done! I've updated the test names to be more descriptive, fixed the types, and added the requested assertions for mock calls and ordering.

@devanshuprakashdevanshuprakashforce-pushed thefix/mssql-connection-pool-exhaustion branch from708f39f to5c110e8CompareDecember 3, 2025 18:45
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@aqrlnaqrlnaqrln left review comments

@coderabbitaicoderabbitai[bot]coderabbitai[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

7.2.0

Development

Successfully merging this pull request may close these issues.

Prisma with @prisma/adapter-mssql: Connection Pool Exhaustion and "EREQINPROG" Errors in Production

2 participants

@devanshuprakash@aqrln
[8]ページ先頭
©2009-2025 Movatter.jp