Skip to content

Commit 512e011

Browse files
N2D4N2D4
committed
Fix flakey tests
1 parent d9acb30 commit 512e011

File tree

5 files changed

+37
-19
lines changed

5 files changed

+37
-19
lines changed

‎.vscode/settings.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
"quetzallabs",
7373
"rehype",
7474
"reqs",
75+
"retryable",
7576
"RPID",
7677
"simplewebauthn",
7778
"spoofable",

‎apps/backend/src/app/api/latest/integrations/neon/oauth/idp/[[...route]]/idp.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,18 @@ function createAdapter(options: {
1919
model: string,
2020
idOrWhere: string | { propertyKey: keyof AdapterPayload, propertyValue: string },
2121
updater: (old: AdapterData | undefined) => AdapterData | undefined
22-
) => void | Promise<void>,
22+
) => Promise<AdapterData | undefined>,
2323
}): AdapterConstructor {
2424
const niceUpdate = async (
2525
model: string,
2626
idOrWhere: string | { propertyKey: keyof AdapterPayload, propertyValue: string },
2727
updater?: (old: AdapterData | undefined) => AdapterData | undefined,
2828
): Promise<AdapterPayload | undefined> => {
29-
let wasCalled = false as boolean; // casting due to https://stackoverflow.com/a/76698580
30-
let updated: AdapterData | undefined;
31-
await options.onUpdateUnique(
29+
const updated = await options.onUpdateUnique(
3230
model,
3331
idOrWhere,
34-
(old) => {
35-
if (wasCalled) throw new StackAssertionError('Adapter update called more than once');
36-
wasCalled = true;
37-
updated = (updater ? updater(old) : old);
38-
return updated;
39-
},
32+
updater ? updater : (old) => old,
4033
);
41-
if (!wasCalled) throw new StackAssertionError('Adapter update was not called');
4234
return updated?.payload;
4335
};
4436

@@ -94,7 +86,7 @@ function createAdapter(options: {
9486
function createPrismaAdapter(idpId: string) {
9587
return createAdapter({
9688
async onUpdateUnique(model, idOrWhere, updater) {
97-
await retryTransaction(async (tx) => {
89+
return await retryTransaction(async (tx) => {
9890
const oldAll = await tx.idPAdapterData.findMany({
9991
where: typeof idOrWhere === 'string' ? {
10092
idpId,
@@ -163,6 +155,8 @@ function createPrismaAdapter(idpId: string) {
163155
});
164156
}
165157
}
158+
159+
return updated;
166160
});
167161
},
168162
});

‎apps/backend/src/prisma-client.tsx

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ if (getNodeEnvironment() !== 'production') {
2020
globalForPrisma.prisma = prismaClient;
2121
}
2222

23+
class TransactionErrorThatShouldBeRetried extends Error {
24+
constructor(...args: ConstructorParameters<typeof Error>) {
25+
super(...args);
26+
this.name = 'TransactionErrorThatShouldBeRetried';
27+
}
28+
}
2329

2430
export async function retryTransaction<T>(fn: (...args: Parameters<Parameters<typeof prismaClient.$transaction>[0]>) => Promise<T>): Promise<T> {
2531
// disable serializable transactions for now, later we may re-add them
@@ -30,22 +36,34 @@ export async function retryTransaction<T>(fn: (...args: Parameters<Parameters<ty
3036
return await traceSpan(`transaction attempt #${attemptIndex}`, async (attemptSpan) => {
3137
const attemptRes = await (async () => {
3238
try {
33-
return await prismaClient.$transaction(async (...args) => {
39+
return await prismaClient.$transaction(async (tx, ...args) => {
3440
try {
35-
return Result.ok(await fn(...args));
41+
const res = await fn(tx, ...args);
42+
if (getNodeEnvironment() === 'development' || getNodeEnvironment() === 'test') {
43+
// In dev/test, let's just fail the transaction with a certain probability, if we haven't already failed multiple times
44+
// this is to test the logic that every transaction is retryable
45+
if (attemptIndex < 3 && Math.random() < 0.5) {
46+
throw new TransactionErrorThatShouldBeRetried("Test error for dev/test. This should automatically be retried.");
47+
}
48+
}
49+
return Result.ok(res);
3650
} catch (e) {
51+
// we can aggressively retry the transaction here, because we know that the error will cause the transaction to be rolled back
3752
if (e instanceof Prisma.PrismaClientKnownRequestError || e instanceof Prisma.PrismaClientUnknownRequestError) {
38-
// retry
39-
return Result.error(e);
53+
throw new TransactionErrorThatShouldBeRetried("Prisma error inside a transaction. The transaction will be rolled back, and it can be retried.", { cause: e });
4054
}
55+
// to make debugging easier, we don't retry errors that are not related to Prisma
4156
throw e;
4257
}
4358
}, {
4459
isolationLevel: enableSerializable && attemptIndex < 4 ? Prisma.TransactionIsolationLevel.Serializable : undefined,
4560
});
4661
} catch (e) {
47-
// we don't want to retry as aggressively here, because the error may have been thrown after the transaction was already committed
62+
// we don't want to retry as aggressively as inside the transaction here, because the error may have been thrown after the transaction was already committed
4863
// so, we select the specific errors that we know are safe to retry
64+
if (e instanceof TransactionErrorThatShouldBeRetried) {
65+
return Result.error(e);
66+
}
4967
if ([
5068
"Transaction failed due to a write conflict or a deadlock. Please retry your transaction",
5169
"Transaction already closed: A commit cannot be executed on an expired transaction. The timeout for this transaction",

‎apps/e2e/tests/js/api-keys.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ it("should be able to create and update user api keys", async ({ expect }) => {
100100
]
101101
`);
102102

103+
}, {
104+
timeout: 40_000,
103105
});
104106

105107
it("should be able to get user by api key from server app", async ({ expect }) => {
@@ -316,6 +318,8 @@ it("should be able to create a team, add an API key, and get the team from the A
316318
// Verify the team details match
317319
expect(teamByApiKey.id).toBe(team.id);
318320
expect(teamByApiKey.displayName).toBe("Test Team");
321+
}, {
322+
timeout: 40_000,
319323
});
320324

321325
it("should not be able to get a user with a team API key", async ({ expect }) => {

‎apps/e2e/tests/js/js-helpers.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AdminProjectUpdateOptions, StackAdminApp, StackClientApp, StackServerApp } from '@stackframe/js';
2+
import { Result } from '@stackframe/stack-shared/dist/utils/results';
23
import { STACK_BACKEND_BASE_URL, STACK_INTERNAL_PROJECT_ADMIN_KEY, STACK_INTERNAL_PROJECT_CLIENT_KEY, STACK_INTERNAL_PROJECT_SERVER_KEY } from '../helpers';
34

45
export async function scaffoldProject(body?: AdminProjectUpdateOptions) {
@@ -13,11 +14,11 @@ export async function scaffoldProject(body?: AdminProjectUpdateOptions) {
1314

1415
const fakeEmail = `${crypto.randomUUID()}@stack-js-test.example.com`;
1516

16-
await internalApp.signUpWithCredential({
17+
Result.orThrow(await internalApp.signUpWithCredential({
1718
email: fakeEmail,
1819
password: "password",
1920
verificationCallbackUrl: "https://stack-js-test.example.com/verify",
20-
});
21+
}));
2122
const adminUser = await internalApp.getUser({
2223
or: 'throw',
2324
});

0 commit comments

Comments
 (0)