mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
1730a4dc2a
* feat(lint): no-io-in-transaction rule + clear existing violations
Adds a custom ESLint rule (eslint-local-rules.js) that flags awaited
external/non-DB I/O inside a Prisma interactive `$transaction(async (tx) =>
…)` callback. Such calls (HTTP fetch, image scanner, Buzz API, Axiom logging,
Redis cache busts, search-index queueing) add their latency to the txn's
wall-clock timeout budget and, when slow, blow it ("Transaction already
closed"). This is the recurring class behind #2375 / #2377 / #2379 — the rule
turns "sweep it again" into "caught in review/IDE".
Detection is a curated denylist of known I/O call names (low false-positive);
calls on the tx client itself (`tx.*`, `tx.$queryRaw`/`$executeRaw`) are
always allowed. Validated with a RuleTester suite (10 cases).
Wiring is conditional in .eslintrc.js: the rule activates automatically once
`eslint-plugin-local-rules` is installed (`pnpm add -D eslint-plugin-local-
rules`) and is skipped until then, so `next lint` keeps working and CI's
`pnpm install --frozen-lockfile` is unaffected (no package.json/lockfile
change in this PR — pnpm wasn't available to regenerate the lockfile).
Brings the codebase to a clean baseline for the rule:
Fixed (moved external work after commit / made fire-and-forget):
- collection/article(x2)/model(x2)/bountyEntry: userXCountCache.refresh()
(Redis) moved to after the txn commits, using the returned row's id.
- referral/redeemableCode: error-branch logToAxiom() de-awaited (Axiom HTTP),
matching the #2379 pattern (.catch retained / added).
Ratchet-disabled with TODO(tx-io) (intentional / needs careful change):
- bounty.createBounty + bountyEntry.awardBountyEntry: Buzz charge/settlement
inside the txn — moving needs charge→tx→refund-on-failure compensation
(a PG rollback can't undo an external Buzz charge); left for a domain owner.
- report.createReport CSAM branch: search-index delete inside the txn —
moving needs hoisting the CSAM guard post-commit on a sensitive path.
tsc --noEmit error-neutral vs baseline across all touched files (pre-existing
Prisma-client-drift errors unchanged; CI regenerates the client).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(lint): audit follow-ups — install plugin, warn-level, create-only refresh, tests
Addresses the audit of #2382:
- H1: install `eslint-plugin-local-rules` (devDep + lockfile via pnpm
--lockfile-only) and wire the rule unconditionally. Previously the rule was
only activated when the plugin happened to resolve, but the 6
`// eslint-disable-next-line local-rules/no-io-in-transaction` directives
error with "Definition for rule not found" in ESLint 8 when the rule is
unconfigured — so `next lint` broke in the plugin-absent state. With the
plugin now a real dependency the rule is configured and the directives are
valid.
- Rule severity set to `warn` (not `error`): surfaces in the editor / next lint
as a guardrail without failing lint or the build; escalate later.
- M1: bountyEntry.upsertBountyEntry count-cache refresh is now gated to the
create path (`!id`). The pre-move code only refreshed in the create branch;
the first move ran it on updates too (extra primary-DB COUNT + Redis on every
description edit). Restored create-only semantics.
- Test coverage: add src/server/services/__tests__/no-io-in-transaction.test.ts
(RuleTester via vitest, 20 cases incl. FP/FN regression guards). Runs with
`pnpm test:unit:run`; 20/20 pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* ci(pr-check): gate the no-io-in-transaction rule via its RuleTester suite
Adds a `test:lint-rules` script (vitest run of the rule's test) and a
"Test lint rules" step to the pr-check workflow, right after typecheck (reuses
the job's already-installed deps).
Scoped to the rule's own test rather than the full `test:unit:run` suite: the
full suite currently has pre-existing failures (e.g. a timezone-dependent
redeemableCode date assertion) and isn't CI-green, so wiring it wholesale would
block all PRs. This step deterministically gates the custom rule's correctness;
broadening to the full suite is a separate cleanup once those failures are fixed.
Note: the rule itself is `warn`-level, so `next lint` surfaces violations
without failing the build — this CI step gates the RULE (regressions in
eslint-local-rules.js), not new violations.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* ci: retrigger preview (prior build hit transient pnpm-install network timeouts)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
181 lines
6.5 KiB
JavaScript
181 lines
6.5 KiB
JavaScript
/**
|
|
* Local ESLint rules for civitai.
|
|
*
|
|
* Loaded via `eslint-plugin-local-rules` (referenced as the `local-rules`
|
|
* plugin in .eslintrc.js). Add new rules to the exported object below.
|
|
*/
|
|
|
|
'use strict';
|
|
|
|
/**
|
|
* no-io-in-transaction
|
|
*
|
|
* Flags awaited external / non-database I/O inside a Prisma interactive
|
|
* transaction callback — `db.$transaction(async (tx) => { ... })`.
|
|
*
|
|
* Interactive transactions hold a DB connection open under a wall-clock
|
|
* timeout (Prisma default 5000ms, or an explicit `{ timeout }`). An awaited
|
|
* network call inside the callback (HTTP fetch, image scanner, Buzz API,
|
|
* Axiom logging, Redis cache busts, search-index queueing, …) adds its latency
|
|
* to that budget and, when slow, blows it: "Transaction already closed: a
|
|
* commit cannot be executed on an expired transaction". A Postgres rollback
|
|
* also can't undo external side effects, so the atomicity is usually illusory.
|
|
*
|
|
* Fix: do the external work AFTER the transaction commits (return the needed
|
|
* ids from the callback, then act on them), or make pure-logging calls
|
|
* fire-and-forget. See PRs #2375 / #2377 / #2379 for the established pattern.
|
|
*
|
|
* Detection is a curated denylist of known I/O call names (low false-positive,
|
|
* extend as new I/O helpers appear). Calls on the transaction client itself
|
|
* (`tx.*`, including `tx.$queryRaw` / `tx.$executeRaw`) are always allowed.
|
|
* Intentional exceptions should use:
|
|
* // eslint-disable-next-line local-rules/no-io-in-transaction -- <reason>
|
|
*/
|
|
const IO_CALL_NAMES = new Set([
|
|
// HTTP / generic
|
|
'fetch',
|
|
// image ingestion / scanner
|
|
'ingestImage',
|
|
'ingestImageBulk',
|
|
'createImageIngestionRequest',
|
|
// orchestrator
|
|
'submitWorkflow',
|
|
// Buzz / payments (external ledger via buzzApiFetch)
|
|
'buzzApiFetch',
|
|
'createBuzzTransaction',
|
|
'createBuzzTransactionMany',
|
|
'createMultiAccountBuzzTransaction',
|
|
'refundTransaction',
|
|
'refundMultiAccountTransaction',
|
|
'getMultiAccountTransactionsByPrefix',
|
|
'deleteBidsForModelVersion',
|
|
// observability (Axiom HTTP ingest)
|
|
'logToAxiom',
|
|
// search index + redis cache (network)
|
|
'queueUpdate',
|
|
'updateDocs',
|
|
'refresh', // *Cache.refresh(...) — Redis + cross-pool read
|
|
'bust', // bustMvCache etc.
|
|
'bustMvCache',
|
|
'invalidateManyImageExistence',
|
|
// email
|
|
'sendEmail',
|
|
]);
|
|
|
|
// Promise-combinator wrappers whose argument we should unwrap to find the
|
|
// underlying call (e.g. `await foo().catch(() => null)` -> inspect `foo()`).
|
|
const PASSTHROUGH_MEMBERS = new Set(['catch', 'then', 'finally']);
|
|
|
|
/** Walk a member chain to its root object identifier name (e.g. tx.user.x -> "tx"). */
|
|
function rootObjectName(node) {
|
|
let cur = node;
|
|
while (cur && cur.type === 'MemberExpression') cur = cur.object;
|
|
if (cur && cur.type === 'CallExpression') return rootObjectName(cur.callee);
|
|
return cur && cur.type === 'Identifier' ? cur.name : null;
|
|
}
|
|
|
|
/** Given a CallExpression, return the called name (identifier or member property). */
|
|
function calleeName(callExpr) {
|
|
const callee = callExpr.callee;
|
|
if (!callee) return null;
|
|
if (callee.type === 'Identifier') return callee.name;
|
|
if (callee.type === 'MemberExpression' && callee.property) {
|
|
return callee.property.type === 'Identifier' ? callee.property.name : null;
|
|
}
|
|
return null;
|
|
}
|
|
|
|
/**
|
|
* Resolve the "effective" I/O call inside an awaited expression, unwrapping
|
|
* `.catch()/.then()/.finally()` passthroughs. Returns { name, node } or null.
|
|
*/
|
|
function resolveIoCall(expr, txParamNames) {
|
|
if (!expr || expr.type !== 'CallExpression') return null;
|
|
const name = calleeName(expr);
|
|
|
|
// Unwrap promise passthroughs: await foo().catch(...) -> inspect foo()
|
|
if (
|
|
name &&
|
|
PASSTHROUGH_MEMBERS.has(name) &&
|
|
expr.callee.type === 'MemberExpression' &&
|
|
expr.callee.object
|
|
) {
|
|
return resolveIoCall(expr.callee.object, txParamNames);
|
|
}
|
|
|
|
// Allow calls on the transaction client itself: tx.*(...), tx.$queryRaw`...`
|
|
const root = rootObjectName(expr.callee);
|
|
if (root && txParamNames.has(root)) return null;
|
|
|
|
if (name && IO_CALL_NAMES.has(name)) return { name, node: expr };
|
|
return null;
|
|
}
|
|
|
|
const noIoInTransaction = {
|
|
meta: {
|
|
type: 'problem',
|
|
docs: {
|
|
description:
|
|
'Disallow awaited external/network I/O inside a Prisma interactive $transaction callback (blows the txn timeout budget).',
|
|
recommended: true,
|
|
},
|
|
schema: [],
|
|
messages: {
|
|
ioInTx:
|
|
"Awaited '{{name}}(...)' performs external I/O inside a $transaction callback — it consumes the transaction's timeout budget. Do this after the transaction commits, or make it fire-and-forget. If intentional, add: // eslint-disable-next-line local-rules/no-io-in-transaction -- <reason>",
|
|
},
|
|
},
|
|
create(context) {
|
|
// Stack of active transaction-callback contexts. Each entry holds the set
|
|
// of param names treated as the tx client (usually just {"tx"}).
|
|
const txStack = [];
|
|
// Function nodes that are transaction callbacks -> their tx param name set.
|
|
const txCallbackFns = new WeakMap();
|
|
|
|
function isTransactionCall(node) {
|
|
return (
|
|
node.type === 'CallExpression' &&
|
|
node.callee.type === 'MemberExpression' &&
|
|
node.callee.property &&
|
|
node.callee.property.type === 'Identifier' &&
|
|
node.callee.property.name === '$transaction' &&
|
|
node.arguments.length > 0 &&
|
|
(node.arguments[0].type === 'ArrowFunctionExpression' ||
|
|
node.arguments[0].type === 'FunctionExpression')
|
|
);
|
|
}
|
|
|
|
return {
|
|
CallExpression(node) {
|
|
if (!isTransactionCall(node)) return;
|
|
const fn = node.arguments[0];
|
|
const params = new Set();
|
|
const first = fn.params && fn.params[0];
|
|
if (first && first.type === 'Identifier') params.add(first.name);
|
|
txCallbackFns.set(fn, params);
|
|
},
|
|
// Track entering/leaving any function so we know if we're lexically
|
|
// inside a transaction callback (including nested arrows/maps).
|
|
':function'(node) {
|
|
if (txCallbackFns.has(node)) txStack.push(txCallbackFns.get(node));
|
|
else if (txStack.length) txStack.push(txStack[txStack.length - 1]);
|
|
},
|
|
':function:exit'(node) {
|
|
if (txStack.length) txStack.pop();
|
|
},
|
|
AwaitExpression(node) {
|
|
if (txStack.length === 0) return;
|
|
const txParamNames = txStack[txStack.length - 1];
|
|
const io = resolveIoCall(node.argument, txParamNames);
|
|
if (io) {
|
|
context.report({ node: io.node, messageId: 'ioInTx', data: { name: io.name } });
|
|
}
|
|
},
|
|
};
|
|
},
|
|
};
|
|
|
|
module.exports = {
|
|
'no-io-in-transaction': noIoInTransaction,
|
|
};
|