Cancel in-flight Tedious requests when their Effects are interrupted

[fubhy]

Summary

Interrupting a query can return its connection to the pool while Tedious is still executing on that connection.

Important

This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.

Request interruption does not cancel Tedious

Module: mssql/MssqlClient
Audit ID: sql-adapters-ms-3
Severity / confidence: high / high

What happens

Interrupting a query can return its connection to the pool while Tedious is still executing on that connection.

Why it happens

The Effect.callback bridges return no interruption canceler; conn.cancel() is called only before request execution, not when the fiber is interrupted.

Expected behavior

Interruption must stop or quarantine an in-flight request before its scoped pool lease can be reused.

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

• packages/sql/mssql/src/MssqlClient.ts:336-376
• packages/sql/mssql/src/MssqlClient.ts:378-424

View problematic code at packages/sql/mssql/src/MssqlClient.ts:336-376

      const run = (
        sql: string,
        values?: ReadonlyArray<any>,
        rowsAsArray = false
      ) =>
        Effect.callback<any, SqlError>((resume) => {
          const req = new Tedious.Request(sql, (cause, _rowCount, result) => {
            if (cause) {
              resume(
                Effect.fail(new SqlError({ reason: classifyError(cause, "Failed to execute statement", "execute") }))
              )
              return
            }

            if (rowsAsArray) {
              result = result.map((row: any) => row.map((_: any) => _.value))
            } else {
              result = rowsToObjects(result)
            }

            resume(Effect.succeed(result))
          })

          if (values) {
            for (let i = 0, len = values.length; i < len; i++) {
              const value = values[i]
              const name = numberToParamName(i)

              if (isMssqlParam(value)) {
                req.addParameter(name, value.paramA, value.paramB, value.paramC)
              } else {
                const kind = Statement.primitiveKind(value)
                const type = parameterTypes[kind]
                req.addParameter(name, type, value)
              }
            }
          }

          conn.cancel()
          conn.execSql(req)
        })

View exact lines on GitHub

View problematic code at packages/sql/mssql/src/MssqlClient.ts:378-424

      const runProcedure = (
        procedure: Procedure.ProcedureWithValues<any, any, any>,
        transformRows: ((rows: ReadonlyArray<any>) => ReadonlyArray<any>) | undefined
      ) =>
        Effect.callback<any, SqlError>((resume) => {
          const result: Record<string, any> = {}

          const req = new Tedious.Request(
            escape(procedure.name),
            (cause, _, rows) => {
              if (cause) {
                resume(
                  Effect.fail(new SqlError({ reason: classifyError(cause, "Failed to execute statement", "execute") }))
                )
              } else {
                rows = rowsToObjects(rows)
                if (transformRows) {
                  rows = transformRows(rows) as any
                }
                resume(
                  Effect.succeed({
                    params: result,
                    rows
                  })
                )
              }
            }
          )

          for (const name in procedure.params) {
            const param = procedure.params[name]
            const value = procedure.values[name]
            req.addParameter(name, param.type, value, param.options)
          }

          for (const name in procedure.outputParams) {
            const param = procedure.outputParams[name]
            req.addOutputParameter(name, param.type, undefined, param.options)
          }

          req.on("returnValue", (name, value) => {
            Rec.assignProperty(result, name, value)
          })

          conn.cancel()
          conn.callProcedure(req)
        })

View exact lines on GitHub

Reproduction

pnpm test --run packages/sql/mssql/test/Client.test.ts

Observed failure: FAIL: interruption made no additional Tedious cancel call.

Implementation handoff

The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.

1. Start with the pinned implementation excerpts and the Why it happens analysis above.
2. Change the implementation so it satisfies the stated Expected behavior; do not weaken or remove the reproduction assertions.
3. Run the focused reproduction command(s) and confirm the observed failures become passing tests:

pnpm test --run packages/sql/mssql/test/Client.test.ts

4. Run the affected package's existing tests, then the repository lint and type checks before requesting review.

Audit provenance

• Audit base: c9b56ab507f224426ee8388dc450da447ec4715f
• Reproduction base: c9b56ab507f224426ee8388dc450da447ec4715f
• Findings: sql-adapters-ms-3
• Initial patch: focused reproduction tests; implementation fix pending

Closes EFF-436

[changeset-bot]

🦋 Changeset detected

Latest commit: 938d379

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
@effect/sql-mssql	Patch
effect	Patch
@effect/opentelemetry	Patch
@effect/platform-browser	Patch
@effect/platform-bun	Patch
@effect/platform-deno	Patch
@effect/platform-node-shared	Patch
@effect/platform-node	Patch
@effect/vitest	Patch
@effect/ai-anthropic	Patch
@effect/ai-openai-compat	Patch
@effect/ai-openai	Patch
@effect/ai-openrouter	Patch
@effect/atom-react	Patch
@effect/atom-solid	Patch
@effect/atom-vue	Patch
@effect/sql-clickhouse	Patch
@effect/sql-d1	Patch
@effect/sql-libsql	Patch
@effect/sql-mysql2	Patch
@effect/sql-pg	Patch
@effect/sql-pglite	Patch
@effect/sql-sqlite-bun	Patch
@effect/sql-sqlite-do	Patch
@effect/sql-sqlite-node	Patch
@effect/sql-sqlite-react-native	Patch
@effect/sql-sqlite-wasm	Patch
@effect/docgen	Patch
@effect/doctest	Patch
@effect/openapi-generator	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pullfrog]

Caution

This PR only adds the failing regression test; the implementation fix is still missing, so the branch cannot merge. The reproduction itself is sound, but it needs to be accompanied by the source change before approval.

Reviewed changes

This PR adds one regression test to packages/sql/mssql/test/Client.test.ts that asserts conn.cancel() is called when an in-flight Tedious request is interrupted. It does not yet modify packages/sql/mssql/src/MssqlClient.ts.

• Added a vi.mock of tedious with a MockConnection whose cancel() call count is observable.
• Added it.effect("cancels an in-flight Tedious request when interrupted", ...) that forks a long query, yields, interrupts the child fiber, and asserts cancelCalls incremented.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

.

[pullfrog]

✅ No new issues found.

Reviewed changes

This incremental review covered the implementation fix pushed since the prior review.

• Added the missing Effect.callback cleanup in packages/sql/mssql/src/MssqlClient.ts: both run and runProcedure now return Effect.sync(() => conn.cancel()), so interruption cancels the in-flight Tedious request before the scoped pool lease can be reused.
• Updated the regression test in packages/sql/mssql/test/Client.test.ts so the mock request stays in-flight (completeRequests = false after constructing the client), ensuring the cancel() call comes from the interruption cleanup path rather than from a synchronous request callback.
• Added a changeset (.changeset/cancel-tedious-requests.md) describing the bugfix.

The prior review feedback has been addressed: the implementation now wires interruption to conn.cancel() and the test verifies the cleanup path rather than coincidental callback behavior.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}