Skip to content

Commit 2b8bb8f

Browse files
committed
fix: more issues found with formbricks
- Nested transaction handling - Fixed issue with concurrent transactions - Temporarily disabled running Kysely query interceptor in a transaction
1 parent ab0cfff commit 2b8bb8f

File tree

17 files changed

+734
-625
lines changed

17 files changed

+734
-625
lines changed

TODO.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@
5151
- [x] Count
5252
- [x] Aggregate
5353
- [x] Group by
54-
- [ ] Raw queries
54+
- [x] Raw queries
55+
- [ ] Transactions
56+
- [x] Interactive transaction
57+
- [ ] Batch transaction
5558
- [ ] Extensions
5659
- [x] Query builder API
5760
- [x] Computed fields
@@ -69,6 +72,8 @@
6972
- [x] Custom field name
7073
- [ ] Strict undefined checks
7174
- [ ] Benchmark
75+
- [ ] Plugin
76+
- [ ] Post-mutation hooks should be called after transaction is committed
7277
- [ ] Polymorphism
7378
- [ ] Validation
7479
- [ ] Access Policy

packages/runtime/src/client/client-impl.ts

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { lowerCaseFirst } from '@zenstackhq/common-helpers';
2-
import type { SqliteDialectConfig } from 'kysely';
2+
import type { QueryExecutor, SqliteDialectConfig } from 'kysely';
33
import {
44
CompiledQuery,
55
DefaultConnectionProvider,
@@ -60,6 +60,7 @@ export class ClientImpl<Schema extends SchemaDef> {
6060
private readonly schema: Schema,
6161
private options: ClientOptions<Schema>,
6262
baseClient?: ClientImpl<Schema>,
63+
executor?: QueryExecutor,
6364
) {
6465
this.$schema = schema;
6566
this.$options = options ?? ({} as ClientOptions<Schema>);
@@ -73,22 +74,24 @@ export class ClientImpl<Schema extends SchemaDef> {
7374
if (baseClient) {
7475
this.kyselyProps = {
7576
...baseClient.kyselyProps,
76-
executor: new ZenStackQueryExecutor(
77-
this,
78-
baseClient.kyselyProps.driver as ZenStackDriver,
79-
baseClient.kyselyProps.dialect.createQueryCompiler(),
80-
baseClient.kyselyProps.dialect.createAdapter(),
81-
new DefaultConnectionProvider(baseClient.kyselyProps.driver),
82-
),
77+
executor:
78+
executor ??
79+
new ZenStackQueryExecutor(
80+
this,
81+
baseClient.kyselyProps.driver as ZenStackDriver,
82+
baseClient.kyselyProps.dialect.createQueryCompiler(),
83+
baseClient.kyselyProps.dialect.createAdapter(),
84+
new DefaultConnectionProvider(baseClient.kyselyProps.driver),
85+
),
8386
};
8487
this.kyselyRaw = baseClient.kyselyRaw;
88+
this.auth = baseClient.auth;
8589
} else {
8690
const dialect = this.getKyselyDialect();
8791
const driver = new ZenStackDriver(dialect.createDriver(), new Log(this.$options.log ?? []));
8892
const compiler = dialect.createQueryCompiler();
8993
const adapter = dialect.createAdapter();
9094
const connectionProvider = new DefaultConnectionProvider(driver);
91-
const executor = new ZenStackQueryExecutor(this, driver, compiler, adapter, connectionProvider);
9295

9396
this.kyselyProps = {
9497
config: {
@@ -97,7 +100,7 @@ export class ClientImpl<Schema extends SchemaDef> {
97100
},
98101
dialect,
99102
driver,
100-
executor,
103+
executor: executor ?? new ZenStackQueryExecutor(this, driver, compiler, adapter, connectionProvider),
101104
};
102105

103106
// raw kysely instance with default executor
@@ -112,14 +115,21 @@ export class ClientImpl<Schema extends SchemaDef> {
112115
return createClientProxy(this);
113116
}
114117

115-
public get $qb() {
118+
get $qb() {
116119
return this.kysely;
117120
}
118121

119-
public get $qbRaw() {
122+
get $qbRaw() {
120123
return this.kyselyRaw;
121124
}
122125

126+
/**
127+
* Create a new client with a new query executor.
128+
*/
129+
withExecutor(executor: QueryExecutor) {
130+
return new ClientImpl(this.schema, this.$options, this, executor);
131+
}
132+
123133
private getKyselyDialect() {
124134
return match(this.schema.provider.type)
125135
.with('sqlite', () => this.makeSqliteKyselyDialect())
@@ -136,11 +146,17 @@ export class ClientImpl<Schema extends SchemaDef> {
136146
}
137147

138148
async $transaction<T>(callback: (tx: ClientContract<Schema>) => Promise<T>): Promise<T> {
139-
return this.kysely.transaction().execute((tx) => {
140-
const txClient = new ClientImpl<Schema>(this.schema, this.$options);
141-
txClient.kysely = tx;
142-
return callback(txClient as unknown as ClientContract<Schema>);
143-
});
149+
if (this.kysely.isTransaction) {
150+
// proceed directly if already in a transaction
151+
return callback(this as unknown as ClientContract<Schema>);
152+
} else {
153+
// otherwise, create a new transaction, clone the client, and execute the callback
154+
return this.kysely.transaction().execute((tx) => {
155+
const txClient = new ClientImpl<Schema>(this.schema, this.$options);
156+
txClient.kysely = tx;
157+
return callback(txClient as unknown as ClientContract<Schema>);
158+
});
159+
}
144160
}
145161

146162
get $procedures() {

packages/runtime/src/client/crud/operations/base.ts

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import {
66
ExpressionWrapper,
77
sql,
88
UpdateResult,
9+
type IsolationLevel,
910
type Expression as KyselyExpression,
1011
type SelectQueryBuilder,
1112
} from 'kysely';
1213
import { nanoid } from 'nanoid';
14+
import { inspect } from 'node:util';
1315
import { match } from 'ts-pattern';
1416
import { ulid } from 'ulid';
1517
import * as uuid from 'uuid';
@@ -203,7 +205,11 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
203205
result = await query.execute();
204206
} catch (err) {
205207
const { sql, parameters } = query.compile();
206-
throw new QueryError(`Failed to execute query: ${err}, sql: ${sql}, parameters: ${parameters}`);
208+
let message = `Failed to execute query: ${err}, sql: ${sql}`;
209+
if (this.options.debug) {
210+
message += `, parameters: \n${parameters.map((p) => inspect(p)).join('\n')}`;
211+
}
212+
throw new QueryError(message, err);
207213
}
208214

209215
if (inMemoryDistinct) {
@@ -1181,18 +1187,13 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
11811187

11821188
query = query.modifyEnd(this.makeContextComment({ model, operation: 'update' }));
11831189

1184-
try {
1185-
if (!returnData) {
1186-
const result = await query.executeTakeFirstOrThrow();
1187-
return { count: Number(result.numUpdatedRows) } as Result;
1188-
} else {
1189-
const idFields = getIdFields(this.schema, model);
1190-
const result = await query.returning(idFields as any).execute();
1191-
return result as Result;
1192-
}
1193-
} catch (err) {
1194-
const { sql, parameters } = query.compile();
1195-
throw new QueryError(`Error during updateMany: ${err}, sql: ${sql}, parameters: ${parameters}`);
1190+
if (!returnData) {
1191+
const result = await query.executeTakeFirstOrThrow();
1192+
return { count: Number(result.numUpdatedRows) } as Result;
1193+
} else {
1194+
const idFields = getIdFields(this.schema, model);
1195+
const result = await query.returning(idFields as any).execute();
1196+
return result as Result;
11961197
}
11971198
}
11981199

@@ -1900,11 +1901,20 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
19001901
return returnRelation;
19011902
}
19021903

1903-
protected async safeTransaction<T>(callback: (tx: ToKysely<Schema>) => Promise<T>) {
1904+
protected async safeTransaction<T>(
1905+
callback: (tx: ToKysely<Schema>) => Promise<T>,
1906+
isolationLevel?: IsolationLevel,
1907+
) {
19041908
if (this.kysely.isTransaction) {
1909+
// proceed directly if already in a transaction
19051910
return callback(this.kysely);
19061911
} else {
1907-
return this.kysely.transaction().setIsolationLevel('repeatable read').execute(callback);
1912+
// otherwise, create a new transaction and execute the callback
1913+
let txBuilder = this.kysely.transaction();
1914+
if (isolationLevel) {
1915+
txBuilder = txBuilder.setIsolationLevel(isolationLevel);
1916+
}
1917+
return txBuilder.execute(callback);
19081918
}
19091919
}
19101920

packages/runtime/src/client/crud/validator.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { invariant } from '@zenstackhq/common-helpers';
12
import Decimal from 'decimal.js';
23
import stableStringify from 'json-stable-stringify';
34
import { match, P } from 'ts-pattern';
@@ -19,9 +20,8 @@ import {
1920
type UpdateManyArgs,
2021
type UpsertArgs,
2122
} from '../crud-types';
22-
import { InternalError, QueryError } from '../errors';
23+
import { InputValidationError, InternalError, QueryError } from '../errors';
2324
import { fieldHasDefaultValue, getEnum, getModel, getUniqueFields, requireField, requireModel } from '../query-utils';
24-
import { invariant } from '@zenstackhq/common-helpers';
2525

2626
type GetSchemaFunc<Schema extends SchemaDef, Options> = (model: GetModels<Schema>, options: Options) => ZodType;
2727

@@ -179,7 +179,7 @@ export class InputValidator<Schema extends SchemaDef> {
179179
}
180180
const { error } = schema.safeParse(args);
181181
if (error) {
182-
throw new QueryError(`Invalid ${operation} args: ${error.message}`);
182+
throw new InputValidationError(`Invalid ${operation} args: ${error.message}`, error);
183183
}
184184
return args as T;
185185
}
@@ -233,7 +233,7 @@ export class InputValidator<Schema extends SchemaDef> {
233233
private makeWhereSchema(model: string, unique: boolean, withoutRelationFields = false): ZodType {
234234
const modelDef = getModel(this.schema, model);
235235
if (!modelDef) {
236-
throw new QueryError(`Model "${model}" not found`);
236+
throw new QueryError(`Model "${model}" not found in schema`);
237237
}
238238

239239
const fields: Record<string, any> = {};

packages/runtime/src/client/errors.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,33 @@
1+
/**
2+
* Error thrown when input validation fails.
3+
*/
4+
export class InputValidationError extends Error {
5+
constructor(message: string, cause?: unknown) {
6+
super(message, { cause });
7+
}
8+
}
9+
10+
/**
11+
* Error thrown when a query fails.
12+
*/
113
export class QueryError extends Error {
2-
constructor(message: string) {
3-
super(message);
14+
constructor(message: string, cause?: unknown) {
15+
super(message, { cause });
416
}
517
}
618

19+
/**
20+
* Error thrown when an internal error occurs.
21+
*/
722
export class InternalError extends Error {
823
constructor(message: string) {
924
super(message);
1025
}
1126
}
1227

28+
/**
29+
* Error thrown when an entity is not found.
30+
*/
1331
export class NotFoundError extends Error {
1432
constructor(model: string) {
1533
super(`Entity not found for model "${model}"`);

0 commit comments

Comments
 (0)