Skip to content

Commit 4894fe3

Browse files
authored
Merge pull request #2985 from SeedCompany/edgedb/exceptions
2 parents afc45c0 + 7be05f3 commit 4894fe3

File tree

12 files changed

+245
-67
lines changed

12 files changed

+245
-67
lines changed

src/common/exceptions/duplicate.exception.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
import { ArgumentsHost } from '@nestjs/common';
2+
import {
3+
GqlContextType as ContextKey,
4+
GqlExecutionContext,
5+
} from '@nestjs/graphql';
6+
import { lowerCase, upperFirst } from 'lodash';
7+
import type { ExclusivityViolationError } from '~/core/edgedb';
18
import { InputException } from './input.exception';
29

310
/**
@@ -11,4 +18,47 @@ export class DuplicateException extends InputException {
1118
previous,
1219
);
1320
}
21+
22+
static fromDB(exception: ExclusivityViolationError, context?: ArgumentsHost) {
23+
let property = exception.property;
24+
const message = `${upperFirst(
25+
lowerCase(property),
26+
)} already exists and needs to be unique`;
27+
28+
// Attempt to add path prefix automatically to the property name, based
29+
// on given GQL input.
30+
if (context && context.getType<ContextKey>() === 'graphql') {
31+
let gqlArgs = GqlExecutionContext.create(context as any).getArgs();
32+
33+
// unwind single `input` argument, based on our own conventions
34+
if (Object.keys(gqlArgs).length === 1 && 'input' in gqlArgs) {
35+
gqlArgs = gqlArgs.input;
36+
}
37+
38+
const flattened = flattenObject(gqlArgs);
39+
// Guess the correct path based on property name.
40+
// This kinda assumes the property name will be unique amongst all the input.
41+
const guessedPath = Object.keys(flattened).find(
42+
(path) => property === path || path.endsWith('.' + property),
43+
);
44+
property = guessedPath ?? property;
45+
}
46+
47+
const ex = new DuplicateException(property, message, exception);
48+
ex.stack = exception.stack;
49+
return ex;
50+
}
1451
}
52+
53+
const flattenObject = (obj: object, prefix = '') => {
54+
const result: Record<string, any> = {};
55+
for (const [key, value] of Object.entries(obj)) {
56+
if (value && typeof value === 'object' && !Array.isArray(value)) {
57+
const nestedObj = flattenObject(value, prefix + key + '.');
58+
Object.assign(result, nestedObj);
59+
} else {
60+
result[prefix + key] = value;
61+
}
62+
}
63+
return result;
64+
};

src/common/exceptions/exception.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ export const hasPrevious = (e: Error): e is Error & { previous: Error } =>
2929
export function getPreviousList(ex: Error, includeSelf: boolean) {
3030
const previous: Error[] = includeSelf ? [ex] : [];
3131
let current = ex;
32-
while (hasPrevious(current)) {
33-
current = current.previous;
32+
while (current.cause instanceof Error || hasPrevious(current)) {
33+
current = hasPrevious(current)
34+
? current.previous
35+
: (current.cause as Error);
3436
previous.push(current);
3537
}
3638
return previous;

src/components/authentication/authentication.edgedb.repository.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ export class AuthenticationEdgeDBRepository
108108
),
109109
}));
110110
const result = await this.db.run(query);
111+
if (!result) {
112+
return undefined;
113+
}
111114
return {
112115
userId: result?.user?.id,
113116
roles: result?.user?.scopedRoles ?? [],

src/components/user/user.edgedb.repository.ts

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
import { Injectable } from '@nestjs/common';
2-
import {
3-
DuplicateException,
4-
ID,
5-
NotFoundException,
6-
ServerException,
7-
Session,
8-
} from '~/common';
9-
import { e, EdgeDB, isExclusivityViolation } from '~/core/edgedb';
2+
import { ID, NotFoundException, Session } from '~/common';
3+
import { e, EdgeDB } from '~/core/edgedb';
104
import { CreatePerson, User, UserListInput } from './dto';
115
import { UserRepository } from './user.repository';
126

@@ -83,29 +77,14 @@ export class UserEdgedbRepository extends UserRepository {
8377

8478
async create(input: CreatePerson) {
8579
const query = e.insert(e.User, { ...input });
86-
try {
87-
const result = await this.edgedb.run(query);
88-
return result.id;
89-
} catch (e) {
90-
if (isExclusivityViolation(e, 'email')) {
91-
throw new DuplicateException(
92-
'person.email',
93-
'Email address is already in use',
94-
e,
95-
);
96-
}
97-
throw new ServerException('Failed to create user', e);
98-
}
80+
const result = await this.edgedb.run(query);
81+
return result.id;
9982
}
10083

10184
async delete(id: ID, _session: Session, _object: User): Promise<void> {
10285
const query = e.delete(e.User, () => ({
10386
filter_single: { id },
10487
}));
105-
try {
106-
await this.edgedb.run(query);
107-
} catch (exception) {
108-
throw new ServerException('Failed to delete', exception);
109-
}
88+
await this.edgedb.run(query);
11089
}
11190
}

src/core/edgedb/edgedb.service.ts

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Injectable } from '@nestjs/common';
33
import { $, Executor } from 'edgedb';
44
import { retry, RetryOptions } from '~/common/retry';
55
import { TypedEdgeQL } from './edgeql';
6+
import { ExclusivityViolationError } from './exclusivity-violation.error';
67
import { InlineQueryCardinalityMap } from './generated-client/inline-queries';
78
import { OptionsContext, OptionsFn } from './options.context';
89
import { Client } from './reexports';
@@ -58,29 +59,39 @@ export class EdgeDB {
5859
): Promise<R>;
5960

6061
async run(query: any, args?: any) {
61-
if (query instanceof TypedEdgeQL) {
62-
const cardinality = InlineQueryCardinalityMap.get(query.query);
63-
if (!cardinality) {
64-
throw new Error(`Query was not found from inline query generation`);
62+
try {
63+
if (query instanceof TypedEdgeQL) {
64+
const cardinality = InlineQueryCardinalityMap.get(query.query);
65+
if (!cardinality) {
66+
throw new Error(`Query was not found from inline query generation`);
67+
}
68+
const exeMethod = cardinalityToExecutorMethod[cardinality];
69+
70+
return await this.executor.current[exeMethod](query.query, args);
6571
}
66-
const exeMethod = cardinalityToExecutorMethod[cardinality];
6772

68-
return await this.executor.current[exeMethod](query.query, args);
69-
}
73+
if (query.run) {
74+
// eslint-disable-next-line @typescript-eslint/return-await
75+
return await query.run(this.executor.current, args);
76+
}
7077

71-
if (query.run) {
72-
// eslint-disable-next-line @typescript-eslint/return-await
73-
return await query.run(this.executor.current, args);
74-
}
78+
if (typeof query === 'function') {
79+
// eslint-disable-next-line @typescript-eslint/return-await
80+
return await query(this.executor.current, args);
81+
}
7582

76-
if (typeof query === 'function') {
77-
// eslint-disable-next-line @typescript-eslint/return-await
78-
return await query(this.executor.current, args);
79-
}
83+
// For REPL, as this is untyped and assumes many/empty cardinality
84+
if (typeof query === 'string') {
85+
return await this.executor.current.query(query, args);
86+
}
87+
} catch (e) {
88+
// Ignore this call in stack trace. This puts the actual query as the first.
89+
e.stack = e.stack!.replace(/^\s+at EdgeDB\.run.+\n/m, '');
8090

81-
// For REPL, as this is untyped and assumes many/empty cardinality
82-
if (typeof query === 'string') {
83-
return await this.executor.current.query(query, args);
91+
if (ExclusivityViolationError.is(e)) {
92+
throw ExclusivityViolationError.cast(e);
93+
}
94+
throw e;
8495
}
8596

8697
throw new Error('Could not figure out how to run given query');

src/core/edgedb/error.util.ts

Lines changed: 0 additions & 5 deletions
This file was deleted.
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { ConstraintViolationError } from 'edgedb';
2+
3+
export class ExclusivityViolationError extends ConstraintViolationError {
4+
constructor(
5+
readonly objectFQN: string,
6+
readonly property: string,
7+
message?: string,
8+
options?: {
9+
cause?: unknown;
10+
},
11+
) {
12+
super(message, options);
13+
}
14+
15+
static is(e: unknown): e is ConstraintViolationError {
16+
return (
17+
e instanceof ConstraintViolationError &&
18+
(e as any)._message.endsWith(' violates exclusivity constraint')
19+
);
20+
}
21+
22+
static cast(e: ConstraintViolationError) {
23+
if (e instanceof ExclusivityViolationError) {
24+
return e;
25+
}
26+
27+
// @ts-expect-error it's a private field
28+
const message: string = e._message;
29+
// @ts-expect-error it's a private field
30+
const query: string = e._query;
31+
// @ts-expect-error it's a private field
32+
const attrs: Map<number, Uint8Array> = e._attrs;
33+
34+
const detail = new TextDecoder('utf8').decode(attrs.get(2 /* details */));
35+
const matches = detail.match(
36+
/^property '(.+)' of object type '(.+)' violates exclusivity constraint$/,
37+
);
38+
if (!matches) {
39+
throw new Error(
40+
`Could not parse exclusivity violation error; details: ${detail}`,
41+
);
42+
}
43+
const property = matches[1];
44+
const fqn = matches[2];
45+
const ex = new ExclusivityViolationError(fqn, property, message, {
46+
cause: e.cause,
47+
});
48+
49+
ex.stack = e.stack!.replace(
50+
/^ConstraintViolationError:/,
51+
'ExclusivityViolationError:',
52+
);
53+
// @ts-expect-error it's a private field
54+
ex._query = query;
55+
// @ts-expect-error it's a private field
56+
ex._attrs = attrs;
57+
return ex;
58+
}
59+
}

src/core/edgedb/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ export * from './reexports';
22
export { edgeql, EdgeQLArgsOf, EdgeQLReturnOf } from './edgeql';
33
export * from './edgedb.service';
44
export * from './withScope';
5-
export * from './error.util';
5+
export * from './exclusivity-violation.error';

src/core/edgedb/transaction.context.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Injectable, OnModuleDestroy } from '@nestjs/common';
22
import { AsyncLocalStorage } from 'async_hooks';
3-
import { Executor } from 'edgedb';
3+
import { EdgeDBError, Executor } from 'edgedb';
4+
import { getPreviousList } from '~/common';
45
import { Client } from './reexports';
56

67
@Injectable()
@@ -13,9 +14,31 @@ export class TransactionContext
1314
}
1415

1516
async inTx<R>(fn: () => Promise<R>): Promise<R> {
16-
return await this.client.transaction(async (tx) => {
17-
return await this.run(tx, fn);
18-
});
17+
const errorMap = new WeakMap<Error, Error>();
18+
19+
try {
20+
return await this.client.transaction(async (tx) => {
21+
try {
22+
return await this.run(tx, fn);
23+
} catch (error) {
24+
// If the error "wraps" an EdgeDB error, then
25+
// throw that here and save the original.
26+
// This allows EdgeDB to check if the error is retry-able.
27+
// If it is, then this error doesn't matter; otherwise we'll unwrap below.
28+
const maybeRetryableError = getPreviousList(error, true).find(
29+
(e) => e instanceof EdgeDBError,
30+
);
31+
if (maybeRetryableError) {
32+
errorMap.set(maybeRetryableError, error);
33+
throw maybeRetryableError;
34+
}
35+
throw error;
36+
}
37+
});
38+
} catch (error) {
39+
// Unwrap the original error if it was wrapped above.
40+
throw errorMap.get(error) ?? error;
41+
}
1942
}
2043

2144
get current() {

src/core/exception/exception.filter.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class ExceptionFilter implements GqlExceptionFilter {
3232

3333
let normalized: ExceptionJson;
3434
try {
35-
normalized = this.normalizer.normalize(exception);
35+
normalized = this.normalizer.normalize(exception, args);
3636
} catch (e) {
3737
throw exception;
3838
}
@@ -62,6 +62,10 @@ export class ExceptionFilter implements GqlExceptionFilter {
6262
? HttpStatus.FORBIDDEN
6363
: codes.has('Client')
6464
? HttpStatus.BAD_REQUEST
65+
: codes.has('Transient')
66+
? HttpStatus.SERVICE_UNAVAILABLE
67+
: codes.has('NotImplemented')
68+
? HttpStatus.NOT_IMPLEMENTED
6569
: HttpStatus.INTERNAL_SERVER_ERROR;
6670

6771
const out = {

0 commit comments

Comments
 (0)