From 6d1ea26cdc1d334835790ca621efa73881868055 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Sat, 22 Jul 2023 17:32:25 +0800 Subject: [PATCH] refactor(core): improve error handling (#4198) * refactor(core): improve error handling * test(core): add integration tests --- .../middleware/koa-slonik-error-handler.ts | 30 +++------- packages/core/src/oidc/init.ts | 7 ++- .../src/tests/api/admin-user.test.ts | 5 ++ .../api/interaction/error-handling.test.ts | 59 +++++++++++++++++++ .../phrases/src/locales/de/errors/entity.ts | 3 +- .../phrases/src/locales/en/errors/entity.ts | 1 + .../phrases/src/locales/es/errors/entity.ts | 1 + .../phrases/src/locales/fr/errors/entity.ts | 1 + .../phrases/src/locales/it/errors/entity.ts | 1 + .../phrases/src/locales/ja/errors/entity.ts | 1 + .../phrases/src/locales/ko/errors/entity.ts | 1 + .../src/locales/pl-pl/errors/entity.ts | 1 + .../src/locales/pt-br/errors/entity.ts | 1 + .../src/locales/pt-pt/errors/entity.ts | 1 + .../phrases/src/locales/ru/errors/entity.ts | 1 + .../src/locales/tr-tr/errors/entity.ts | 1 + .../src/locales/zh-cn/errors/entity.ts | 9 +-- .../src/locales/zh-hk/errors/entity.ts | 9 +-- .../src/locales/zh-tw/errors/entity.ts | 1 + 19 files changed, 101 insertions(+), 33 deletions(-) create mode 100644 packages/integration-tests/src/tests/api/interaction/error-handling.test.ts diff --git a/packages/core/src/middleware/koa-slonik-error-handler.ts b/packages/core/src/middleware/koa-slonik-error-handler.ts index ad470a506..f5defae7c 100644 --- a/packages/core/src/middleware/koa-slonik-error-handler.ts +++ b/packages/core/src/middleware/koa-slonik-error-handler.ts @@ -1,27 +1,6 @@ -/** - * Slonik Error Types: - * - * BackendTerminatedError, - * CheckIntegrityConstraintViolationError, - * ConnectionError, - * DataIntegrityError, - * ForeignKeyIntegrityConstraintViolationError, - * IntegrityConstraintViolationError, - * InvalidConfigurationError, - * NotFoundError, - * NotNullIntegrityConstraintViolationError, - * StatementCancelledError, - * StatementTimeoutError, - * UnexpectedStateError, - * UniqueIntegrityConstraintViolationError, - * TupleMovedToAnotherPartitionError - * - * (reference)[https://github.com/gajus/slonik#error-handling] - */ - import type { SchemaLike } from '@logto/schemas'; import type { Middleware } from 'koa'; -import { SlonikError, NotFoundError } from 'slonik'; +import { SlonikError, NotFoundError, InvalidInputError } from 'slonik'; import RequestError from '#src/errors/RequestError/index.js'; import { DeletionError, InsertionError, UpdateError } from '#src/errors/SlonikError/index.js'; @@ -35,6 +14,13 @@ export default function koaSlonikErrorHandler(): Middleware { - const user = await findUserById(sub); + // The user may be deleted after the token is issued + const user = await tryThat(findUserById(sub), () => { + throw new errors.InvalidGrant('user not found'); + }); return { accountId: sub, diff --git a/packages/integration-tests/src/tests/api/admin-user.test.ts b/packages/integration-tests/src/tests/api/admin-user.test.ts index a9131bcdd..ab444fe2a 100644 --- a/packages/integration-tests/src/tests/api/admin-user.test.ts +++ b/packages/integration-tests/src/tests/api/admin-user.test.ts @@ -72,6 +72,11 @@ describe('admin console user management', () => { expect(updatedUser).toMatchObject(newUserData); }); + it('should respond 422 when no update data provided', async () => { + const user = await createUserByAdmin(); + await expect(updateUser(user.id, {})).rejects.toMatchObject(createResponseWithCode(422)); + }); + it('should fail when update userinfo with conflict identifiers', async () => { const [username, email, phone] = [generateUsername(), generateEmail(), generatePhone()]; await createUserByAdmin(username, undefined, email, phone); diff --git a/packages/integration-tests/src/tests/api/interaction/error-handling.test.ts b/packages/integration-tests/src/tests/api/interaction/error-handling.test.ts new file mode 100644 index 000000000..e74a3cc8e --- /dev/null +++ b/packages/integration-tests/src/tests/api/interaction/error-handling.test.ts @@ -0,0 +1,59 @@ +/** + * This file contains special error handling test cases of the main flow (sign-in experience) + * that are not belong to any specific flow. + * + * For normal error handling test cases, please add them to the corresponding test files of the flow. + */ + +import { fetchTokenByRefreshToken } from '@logto/js'; +import { SignInIdentifier, InteractionEvent } from '@logto/schemas'; + +import { deleteUser } from '#src/api/admin-user.js'; +import { putInteraction } from '#src/api/interaction.js'; +import { defaultConfig } from '#src/client/index.js'; +import { initClient, processSession } from '#src/helpers/client.js'; +import { enableAllPasswordSignInMethods } from '#src/helpers/sign-in-experience.js'; +import { generateNewUserProfile } from '#src/helpers/user.js'; + +describe('error handling', () => { + it('should throw invalid grant error for token endpoint when user is deleted after sign-in', async () => { + await enableAllPasswordSignInMethods({ + identifiers: [SignInIdentifier.Username], + password: true, + verify: false, + }); + + const { username, password } = generateNewUserProfile({ username: true, password: true }); + const client = await initClient(); + + await client.send(putInteraction, { + event: InteractionEvent.Register, + profile: { + username, + password, + }, + }); + + const { redirectTo } = await client.submitInteraction(); + + const id = await processSession(client, redirectTo); + await deleteUser(id); + + await fetchTokenByRefreshToken( + { + clientId: defaultConfig.appId, + tokenEndpoint: defaultConfig.endpoint + '/oidc/token', + refreshToken: (await client.getRefreshToken())!, + }, + // @ts-expect-error for testing purpose, no need to pass in a real requester + async (...args) => { + const response = await fetch(...args); + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ + error: 'invalid_grant', + error_description: 'grant request is invalid', + }); + } + ); + }); +}); diff --git a/packages/phrases/src/locales/de/errors/entity.ts b/packages/phrases/src/locales/de/errors/entity.ts index 109ab7512..a5d48bca3 100644 --- a/packages/phrases/src/locales/de/errors/entity.ts +++ b/packages/phrases/src/locales/de/errors/entity.ts @@ -1,5 +1,6 @@ const entity = { - create_failed: 'Fehler beim erstellen von {{name}}.', + invalid_input: 'Ungültige Eingabe. Wertliste darf nicht leer sein.', + create_failed: 'Fehler beim Erstellen von {{name}}.', not_exists: '{{name}} existiert nicht.', not_exists_with_id: '{{name}} mit ID `{{id}}` existiert nicht.', not_found: 'Die Ressource wurde nicht gefunden.', diff --git a/packages/phrases/src/locales/en/errors/entity.ts b/packages/phrases/src/locales/en/errors/entity.ts index 6663ab6a5..32c24a77b 100644 --- a/packages/phrases/src/locales/en/errors/entity.ts +++ b/packages/phrases/src/locales/en/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Invalid input. Value list must not be empty.', create_failed: 'Failed to create {{name}}.', not_exists: 'The {{name}} does not exist.', not_exists_with_id: 'The {{name}} with ID `{{id}}` does not exist.', diff --git a/packages/phrases/src/locales/es/errors/entity.ts b/packages/phrases/src/locales/es/errors/entity.ts index 215798c7e..a2af8845d 100644 --- a/packages/phrases/src/locales/es/errors/entity.ts +++ b/packages/phrases/src/locales/es/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Entrada no válida. La lista de valores no debe estar vacía.', create_failed: 'Fallo al crear {{name}}.', not_exists: 'El {{name}} no existe.', not_exists_with_id: 'El {{name}} con ID `{{id}}` no existe.', diff --git a/packages/phrases/src/locales/fr/errors/entity.ts b/packages/phrases/src/locales/fr/errors/entity.ts index 14d3dad63..891874c70 100644 --- a/packages/phrases/src/locales/fr/errors/entity.ts +++ b/packages/phrases/src/locales/fr/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Saisie invalide. La liste des valeurs ne doit pas être vide.', create_failed: 'Échec de la création de {{name}}.', not_exists: "Le {{name}} n'existe pas.", not_exists_with_id: "Le {{name}} avec l'ID `{{id}}` n'existe pas.", diff --git a/packages/phrases/src/locales/it/errors/entity.ts b/packages/phrases/src/locales/it/errors/entity.ts index 6e7b6bac8..460f1753a 100644 --- a/packages/phrases/src/locales/it/errors/entity.ts +++ b/packages/phrases/src/locales/it/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Input non valido. La lista dei valori non deve essere vuota.', create_failed: 'Impossibile creare {{name}}.', not_exists: '{{name}} non esiste.', not_exists_with_id: '{{name}} con ID `{{id}}` non esiste.', diff --git a/packages/phrases/src/locales/ja/errors/entity.ts b/packages/phrases/src/locales/ja/errors/entity.ts index c4fbe291a..e089ba609 100644 --- a/packages/phrases/src/locales/ja/errors/entity.ts +++ b/packages/phrases/src/locales/ja/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: '入力が無効です。値のリストは空であってはなりません。', create_failed: '{{name}}の作成に失敗しました。', not_exists: '{{name}}は存在しません。', not_exists_with_id: 'IDが`{{id}}`の{{name}}は存在しません。', diff --git a/packages/phrases/src/locales/ko/errors/entity.ts b/packages/phrases/src/locales/ko/errors/entity.ts index b083a61cb..d16d0abb7 100644 --- a/packages/phrases/src/locales/ko/errors/entity.ts +++ b/packages/phrases/src/locales/ko/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: '입력이 잘못되었습니다. 값 목록은 비어 있을 수 없습니다.', create_failed: '{{name}} 생성을 실패하였어요.', not_exists: '{{name}}는 존재하지 않아요.', not_exists_with_id: '{{id}} ID를 가진 {{name}}는 존재하지 않아요.', diff --git a/packages/phrases/src/locales/pl-pl/errors/entity.ts b/packages/phrases/src/locales/pl-pl/errors/entity.ts index b7cc21f25..96eed0eac 100644 --- a/packages/phrases/src/locales/pl-pl/errors/entity.ts +++ b/packages/phrases/src/locales/pl-pl/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Nieprawidłowe dane. Lista wartości nie może być pusta.', create_failed: 'Nie udało się utworzyć {{name}}.', not_exists: '{{name}} nie istnieje.', not_exists_with_id: '{{name}} o identyfikatorze `{{id}}` nie istnieje.', diff --git a/packages/phrases/src/locales/pt-br/errors/entity.ts b/packages/phrases/src/locales/pt-br/errors/entity.ts index afe28ead1..b03128ed8 100644 --- a/packages/phrases/src/locales/pt-br/errors/entity.ts +++ b/packages/phrases/src/locales/pt-br/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Entrada inválida. A lista de valores não deve estar vazia.', create_failed: 'Falha ao criar {{name}}.', not_exists: 'O {{name}} não existe.', not_exists_with_id: 'O {{name}} com ID `{{id}}` não existe.', diff --git a/packages/phrases/src/locales/pt-pt/errors/entity.ts b/packages/phrases/src/locales/pt-pt/errors/entity.ts index 762ce6e2c..081bc7f6d 100644 --- a/packages/phrases/src/locales/pt-pt/errors/entity.ts +++ b/packages/phrases/src/locales/pt-pt/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Entrada inválida. A lista de valores não deve estar vazia.', create_failed: 'Falha ao criar {{name}}.', not_exists: '{{name}} não existe.', not_exists_with_id: '{{name}} com o ID `{{id}}` não existe.', diff --git a/packages/phrases/src/locales/ru/errors/entity.ts b/packages/phrases/src/locales/ru/errors/entity.ts index 097d257bb..4f827f1f4 100644 --- a/packages/phrases/src/locales/ru/errors/entity.ts +++ b/packages/phrases/src/locales/ru/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Неверный ввод. Список значений не должен быть пустым.', create_failed: 'Не удалось создать {{name}}.', not_exists: '{{name}} не существует.', not_exists_with_id: '{{name}} с ID `{{id}}` не существует.', diff --git a/packages/phrases/src/locales/tr-tr/errors/entity.ts b/packages/phrases/src/locales/tr-tr/errors/entity.ts index 1c0c9da92..ab49e6e58 100644 --- a/packages/phrases/src/locales/tr-tr/errors/entity.ts +++ b/packages/phrases/src/locales/tr-tr/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: 'Geçersiz giriş. Değer listesi boş olmamalıdır.', create_failed: '{{name}} oluşturulamadı.', not_exists: '{{name}} mevcut değil.', not_exists_with_id: ' `{{id}}` id kimliğine sahip {{name}} mevcut değil.', diff --git a/packages/phrases/src/locales/zh-cn/errors/entity.ts b/packages/phrases/src/locales/zh-cn/errors/entity.ts index 03b04aafc..af8e84bbd 100644 --- a/packages/phrases/src/locales/zh-cn/errors/entity.ts +++ b/packages/phrases/src/locales/zh-cn/errors/entity.ts @@ -1,8 +1,9 @@ const entity = { - create_failed: '创建 {{name}} 失败', - not_exists: '该 {{name}} 不存在', - not_exists_with_id: 'ID 为 `{{id}}` 的 {{name}} 不存在', - not_found: '该资源不存在', + invalid_input: '无效输入。值列表不能为空。', + create_failed: '创建 {{name}} 失败。', + not_exists: '该 {{name}} 不存在。', + not_exists_with_id: 'ID 为 `{{id}}` 的 {{name}} 不存在。', + not_found: '该资源不存在。', }; export default entity; diff --git a/packages/phrases/src/locales/zh-hk/errors/entity.ts b/packages/phrases/src/locales/zh-hk/errors/entity.ts index c790b715a..43e0af1ab 100644 --- a/packages/phrases/src/locales/zh-hk/errors/entity.ts +++ b/packages/phrases/src/locales/zh-hk/errors/entity.ts @@ -1,8 +1,9 @@ const entity = { - create_failed: '創建 {{name}} 失敗', - not_exists: '該 {{name}} 不存在', - not_exists_with_id: 'ID 為 `{{id}}` 的 {{name}} 不存在', - not_found: '該資源不存在', + invalid_input: '無效輸入。值列表不能為空。', + create_failed: '創建 {{name}} 失敗。', + not_exists: '該 {{name}} 不存在。', + not_exists_with_id: 'ID 為 `{{id}}` 的 {{name}} 不存在。', + not_found: '該資源不存在。', }; export default entity; diff --git a/packages/phrases/src/locales/zh-tw/errors/entity.ts b/packages/phrases/src/locales/zh-tw/errors/entity.ts index 5a328c2ec..78c83fee8 100644 --- a/packages/phrases/src/locales/zh-tw/errors/entity.ts +++ b/packages/phrases/src/locales/zh-tw/errors/entity.ts @@ -1,4 +1,5 @@ const entity = { + invalid_input: '無效的輸入。值列表不能為空。', create_failed: '建立 {{name}} 失敗。', not_exists: '{{name}} 不存在。', not_exists_with_id: 'ID 為 `{{id}}` 的 {{name}} 不存在。',