Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions .changeset/eager-writes-open.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,19 @@ not expose.

A context built over a Prisma 8 client whose collections do not cover the
config now refuses at construction rather than quietly downgrading its writes
to non-transactional, and `context.transaction()` over a client that can
neither open a transaction nor join one throws `TransactionUnavailableError`
instead of running the callback with no atomicity. A context assembled from a
hand-built ORM double — `getContext` called without its `client` argument —
still writes directly against that double, and now says so.
to non-transactional. This widens the blast radius of contract drift: before,
a list the client couldn't reach only errored when that list was actually
used (`OrmModelMissingError`, lazily, per operation) while every other list
kept working; now the whole context fails to build — no list is usable —
the moment any one of them is unresolvable. Reviewers judged that the right
trade for #1262: a context that can silently downgrade every write's
atomicity is a worse failure mode than refusing outright, and contract drift
this severe should fail loudly rather than only on the paths a test happens
to exercise.

`context.transaction()` over a client that can neither open a transaction nor
join one throws `TransactionUnavailableError` instead of running the callback
with no atomicity. A context assembled from a hand-built ORM double —
`getContext` called without its `client` argument — still writes directly
against that double, with no such terminal to refuse from; it now warns once
per list and operation via `console.warn` instead of staying silent.
29 changes: 19 additions & 10 deletions packages/core/src/context/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { resolveSyntheticReverseRelation } from '../access/engine.js'
import { ValidationError, DatabaseError } from '../hooks/index.js'
import { databaseErrorMessage, normalizeDatabaseError } from '../lib/prisma-errors.js'
import { nullPrototypeRegistry } from '../lib/null-prototype-registry.js'
import { warnOnce } from '../lib/warn-once.js'
import type { OpenedTransaction, OrmClient, OrmRow, TransactionOpener } from '../access/types.js'
import { createSecuredRead, type SecuredQuery } from '../secured/read.js'
import {
Expand Down Expand Up @@ -141,8 +142,6 @@ export type ServerActionProps =
selectedIds?: string[]
}

const selectWarnings = new Set<string>()

/**
* Warn once per (list, operation) when a caller passes `select` to a read op
* that ignores it. See "Narrowing Reads" in packages/core/CLAUDE.md.
Expand All @@ -154,11 +153,8 @@ function warnIfSelectIgnored(
): void {
if (!args || args.select === undefined) return

const key = `${listName}.${operation}`
if (selectWarnings.has(key)) return
selectWarnings.add(key)

console.warn(
warnOnce(
`select-ignored:${listName}.${operation}`,
`[@opensaas/stack-core] \`select\` is ignored by context.db.${listName}.${operation}() ` +
`and the full (access-filtered) record is returned. ` +
`Narrow a read with \`include\`, or with \`.select()\` on the secured surface, instead. ` +
Expand Down Expand Up @@ -421,6 +417,13 @@ export class OrmHandleUnresolvableError extends Error {
* rather than a handle resolves it here, so the generated context, the test
* harness and `context.transaction()` all hand the engine the same shape.
*
* The returned handle carries no transaction capability and no Unsafe
* surface of its own — both come only from the client itself, which a caller
* passes separately as `getContext`'s own `client` argument. Handing
* `getContext` this handle alone (`getContext(config, requireOrmHandle(config, db.orm), session)`)
* builds a context whose writes run with no rollback guarantee; see the
* `client` parameter's own docblock.
*
* @throws {OrmHandleUnresolvableError} when any declared list is unreachable.
*/
export function requireOrmHandle(config: OpenSaasConfig, orm: OrmRoot): OrmClient {
Expand Down Expand Up @@ -608,9 +611,15 @@ export function getContext<TConfig extends OpenSaasConfig>(
// owner's callback body, carry the deferral registry so writes reached
// through this context join it instead of firing afterTransaction eagerly.
_transactionOwner?: TransactionRegistry,
// The Prisma 8 client the Unsafe surface is built over. Omitted by a caller
// that assembles a context from a hand-built ORM double, whose
// `context.unsafe` then refuses rather than being typed as absent.
// The Prisma 8 client the Unsafe surface AND the write transaction opener
// are both built over. Omitted by a caller that assembles a context from a
// hand-built ORM double: `context.unsafe` then refuses rather than being
// typed as absent, and — the costlier omission — every `context.db` write
// runs with no transaction opener, so it commits directly against the
// handle with no rollback guarantee: exactly the defect #1205 fixed, left
// undiagnosed until now (a `console.warn` names it once per list and
// operation, but nothing stops the write itself). Pass the client whenever
// the caller can.
client?: UnsafeCapableClient,
// Internal: the transaction the Unsafe surface binds its executors to, set
// when rebuilding the context inside `transaction()` (ADR-0056).
Expand Down
24 changes: 24 additions & 0 deletions packages/core/src/context/interactive-transaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,4 +268,28 @@ describe('context.transaction', () => {
BOOT,
)
})

describe('a plain context.db write with no client (#1273)', () => {
test(
'runs anyway, but warns once per list and operation instead of staying silent',
async () => {
const orm = ormClientFor(harness.data, harness.client.orm)
const clientless = getContext(schemaConfig(), orm, { userId: 'u1' })
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})

try {
const created = await clientless.db.User.create({ data: { name: 'jane' } })
expect(created?.name).toBe('jane')
expect(warn).toHaveBeenCalledTimes(1)
expect(warn.mock.calls[0]?.[0]).toContain('no transaction')

await clientless.db.User.create({ data: { name: 'again' } })
expect(warn).toHaveBeenCalledTimes(1)
} finally {
warn.mockRestore()
}
},
BOOT,
)
})
})
25 changes: 25 additions & 0 deletions packages/core/src/context/write-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { hookPipeline } from './hook-pipeline.js'
import { lowerRelationInput, refuseNestedRelationInput } from './relationship-input.js'
import { enumerateInvolvedLists, runWithTransactionBoundary } from './transaction-boundary.js'
import { TransactionRegistry } from '../access/transaction-registry.js'
import { warnOnce } from '../lib/warn-once.js'
// NOTE: `index.ts` imports from this module too — this is an intentional cyclic
// dependency. It is safe because `buildDbDelegate` is only INVOKED at write
// time (never during module evaluation), so by the time it runs the export is
Expand Down Expand Up @@ -124,6 +125,24 @@ async function runInTransaction(
return opener((opened) => fn(opened.ormHandle))
}

/**
* Warn once per (list, operation) when a write has neither an opener of its
* own nor an enclosing transaction to join — a context built without
* `getContext`'s `client` argument (#1273). `context.transaction()` refuses
* this shape outright (`TransactionUnavailableError`); a plain `context.db`
* write has no such terminal to refuse from, so it still runs — directly
* against the handle, with no rollback guarantee — but no longer silently.
*/
function warnNoTransactionCapability(listName: string, operation: WriteOperation): void {
warnOnce(
`no-transaction:${listName}.${operation}`,
`[@opensaas/stack-core] context.db.${listName}.${operation}() is running with no ` +
`transaction and no rollback guarantee: this context was built without getContext's ` +
`\`client\` argument. Pass the Prisma 8 client through (the generated context and ` +
`requireOrmHandle(config, client.orm) both do) to restore it.`,
)
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ListConfig must accept any TypeInfo
function isSingletonList(listConfig: ListConfig<any>): boolean {
return !!listConfig.isSingleton
Expand Down Expand Up @@ -196,6 +215,12 @@ export async function runWritePipeline(args: WritePipelineArgs): Promise<OrmRow
// below it enqueues into.
const existingOwner = context._transactionOwner
const opener = existingOwner ? undefined : context._transactionOpener
// Neither joining an enclosing transaction nor able to open one of its own —
// the silently non-transactional shape #1273 exists to flag (see
// `warnNoTransactionCapability`).
if (existingOwner === undefined && opener === undefined) {
warnNoTransactionCapability(listName, strategy.operation)
}
const ownedRegistry = opener ? new TransactionRegistry() : undefined
const transactionOwnerForBody = existingOwner ?? ownedRegistry

Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/lib/warn-once.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const warned = new Set<string>()

/**
* Log `message` via `console.warn` the first time `key` is seen, and never
* again for that key. Process-lifetime and shared across every caller, so a
* key must be specific enough that two unrelated diagnostics never collide.
*/
export function warnOnce(key: string, message: string): void {
if (warned.has(key)) return
warned.add(key)
console.warn(message)
}
Loading