refactor(core,schemas): refactor the hook library code

refactor the webhooks library code. address some comments

.changeset/fluffy-steaks-flow.md

@@ -4,7 +4,7 @@
 "@logto/console": minor
 ---
 
-refactor the definition of Hook event types:
+refactor the definition of hook event types:
 
-- Add DataHook event types. DataHook events are webhook events that are triggered by data changes.
-- Rename old Hook event types to InteractionHook event types. InteractionHook events are webhook events that are triggered by user interactions.
+- Add `DataHook` event types. `DataHook` are triggered by data changes.
+- Add "interaction" prefix to existing hook event types. Interaction hook events are triggered by end user interactions, e.g. completing sign-in.

.changeset/metal-lions-swim.md

@@ -2,4 +2,7 @@
 "@logto/shared": patch
 ---
 
-add normalizeError method to @logto/shared package. Use this method to normalize error objects for logging. This method is useful for logging errors in a consistent format.
+add `normalizeError` method to `@logto/shared` package.
+
+Use this method to normalize error objects for logging.
+This method is useful for logging errors in a consistent format.

packages/core/src/libraries/hook/index.test.ts

@@ -5,7 +5,7 @@ import { createMockUtils } from '@logto/shared/esm';
 import RequestError from '#src/errors/RequestError/index.js';
 import { mockId, mockIdGenerators } from '#src/test-utils/nanoid.js';
 
-import { DataHookContextManager } from './hook-context-manager.js';
+import { DataHookContextManager } from './context-manager.js';
 import { generateHookTestPayload, parseResponse } from './utils.js';
 
 const { jest } = import.meta;

packages/core/src/libraries/hook/index.ts

@@ -1,14 +1,13 @@
 import {
   LogResult,
   userInfoSelectFields,
-  type DataHookEventPayloadWithoutHookId,
+  type DataHookEventPayload,
   type Hook,
   type HookConfig,
   type HookEvent,
   type HookEventPayload,
-  type HookEventPayloadWithoutHookId,
   type HookTestErrorResponseData,
-  type InteractionHookEventPayloadWithoutHookId,
+  type InteractionHookEventPayload,
 } from '@logto/schemas';
 import { generateStandardId, normalizeError } from '@logto/shared';
 import { conditional, pick, trySafe } from '@silverhand/essentials';
@@ -20,14 +19,20 @@ import { LogEntry } from '#src/middleware/koa-audit-log.js';
 import type Queries from '#src/tenants/Queries.js';
 import { consoleLog } from '#src/utils/console.js';
 
-import { type DataHookContextManager } from './hook-context-manager.js';
+import { type DataHookContextManager } from './context-manager.js';
 import {
   interactionEventToHookEvent,
   type InteractionHookContext,
   type InteractionHookResult,
 } from './types.js';
 import { generateHookTestPayload, parseResponse, sendWebhookRequest } from './utils.js';
 
+type BetterOmit<T, Ignore> = {
+  [key in keyof T as key extends Ignore ? never : key]: T[key];
+};
+
+type HookEventPayloadWithoutHookId = BetterOmit<HookEventPayload, 'hookId'>;
+
 export const createHookLibrary = (queries: Queries) => {
   const {
     applications: { findApplicationById },
@@ -37,59 +42,60 @@ export const createHookLibrary = (queries: Queries) => {
     hooks: { findAllHooks, findHookById },
   } = queries;
 
-  // Send webhook requests with given payloads to the matching hooks. Log the response and error is any.
-  const sendWebhooks = async <T extends HookEventPayloadWithoutHookId>(
-    webhookRequests: Array<{ hooks: Hook[]; payload: T }>
-  ) => {
-    const flatMapRequests = webhookRequests.flatMap(({ hooks, payload }) =>
-      hooks.map((hook) => ({ hook, payload }))
-    );
-
-    return pMap(
-      flatMapRequests,
-      async ({ hook: { id, config, signingKey }, payload }) => {
-        consoleLog.info(`\tTriggering hook ${id} due to ${payload.event} event`);
-
-        const json: HookEventPayload = { ...payload, hookId: id };
-        const logEntry = new LogEntry(`TriggerHook.${payload.event}`);
+  /**
+   * Trigger web hook with the given payload.
+   *
+   * - create audit log for each hook request
+   */
+  const sendWebhook = async (hook: Hook, payload: HookEventPayloadWithoutHookId) => {
+    const { id, config, signingKey } = hook;
+    consoleLog.info(`\tTriggering hook ${id} due to ${payload.event} event`);
 
-        logEntry.append({ hookId: id, hookRequest: { body: json } });
+    const json: HookEventPayload = { ...payload, hookId: id };
+    const logEntry = new LogEntry(`TriggerHook.${payload.event}`);
 
-        // Trigger web hook and log response
-        try {
-          const response = await sendWebhookRequest({
-            hookConfig: config,
-            payload: json,
-            signingKey,
-          });
+    logEntry.append({ hookId: id, hookRequest: { body: json } });
 
-          logEntry.append({
-            response: await parseResponse(response),
-          });
-        } catch (error: unknown) {
-          logEntry.append({
-            result: LogResult.Error,
-            response: conditional(
-              error instanceof HTTPError && (await parseResponse(error.response))
-            ),
-            error: String(normalizeError(error)),
-          });
-        }
+    // Trigger web hook and log response
+    try {
+      const response = await sendWebhookRequest({
+        hookConfig: config,
+        payload: json,
+        signingKey,
+      });
 
-        consoleLog.info(
-          `\tHook ${id} ${logEntry.payload.result === LogResult.Success ? 'succeeded' : 'failed'}`
-        );
+      logEntry.append({
+        response: await parseResponse(response),
+      });
+    } catch (error: unknown) {
+      logEntry.append({
+        result: LogResult.Error,
+        response: conditional(error instanceof HTTPError && (await parseResponse(error.response))),
+        error: String(normalizeError(error)),
+      });
+    }
 
-        await insertLog({
-          id: generateStandardId(),
-          key: logEntry.key,
-          payload: logEntry.payload,
-        });
-      },
-      { concurrency: 10 }
+    consoleLog.info(
+      `\tHook ${id} ${logEntry.payload.result === LogResult.Success ? 'succeeded' : 'failed'}`
     );
+
+    await insertLog({
+      id: generateStandardId(),
+      key: logEntry.key,
+      payload: logEntry.payload,
+    });
   };
 
+  /**
+   * Trigger multiple web hooks with concurrency control.
+   */
+  const sendWebhooks = async <T extends HookEventPayloadWithoutHookId>(
+    webhooks: Array<{ hook: Hook; payload: T }>
+  ) =>
+    pMap(webhooks, async ({ hook, payload }) => sendWebhook(hook, payload), {
+      concurrency: 10,
+    });
+
   /**
    * Trigger interaction hooks with the given interaction context and result.
    */
@@ -127,9 +133,9 @@ export const createHookLibrary = (queries: Queries) => {
       userIp,
       user: user && pick(user, ...userInfoSelectFields),
       application: application && pick(application, 'id', 'type', 'name', 'description'),
-    } satisfies InteractionHookEventPayloadWithoutHookId;
+    } satisfies BetterOmit<InteractionHookEventPayload, 'hookId'>;
 
-    await sendWebhooks([{ hooks, payload }]);
+    await sendWebhooks(hooks.map((hook) => ({ hook, payload })));
   };
 
   /**
@@ -143,7 +149,7 @@ export const createHookLibrary = (queries: Queries) => {
     const found = await findAllHooks();
 
     // Filter hooks that match each events
-    const webhookRequests = hooksManager.contextArray
+    const webhooks = hooksManager.contextArray
       .map(({ event, data }) => {
         const hooks = found.filter(
           ({ event: hookEvent, events, enabled }) =>
@@ -159,13 +165,15 @@ export const createHookLibrary = (queries: Queries) => {
           createdAt: new Date().toISOString(),
           ...hooksManager.metadata,
           ...data,
-        } satisfies DataHookEventPayloadWithoutHookId;
+        } satisfies BetterOmit<DataHookEventPayload, 'hookId'>;
 
         return { hooks, payload };
       })
       .filter(Boolean);
 
-    await sendWebhooks(webhookRequests);
+    await sendWebhooks(
+      webhooks.flatMap(({ hooks, payload }) => hooks.map((hook) => ({ hook, payload })))
+    );
   };
 
   const triggerTestHook = async (hookId: string, events: HookEvent[], config: HookConfig) => {

packages/core/src/middleware/koa-management-api-hooks.ts

@@ -4,7 +4,7 @@ import { type MiddlewareType } from 'koa';
 import { type IRouterParamContext } from 'koa-router';
 
 import { EnvSet } from '#src/env-set/index.js';
-import { DataHookContextManager } from '#src/libraries/hook/hook-context-manager.js';
+import { DataHookContextManager } from '#src/libraries/hook/context-manager.js';
 import {
   buildManagementApiDataHookRegistrationKey,
   hasRegisteredDataHookEvent,

packages/schemas/src/foundations/jsonb-types/hooks.ts

@@ -67,16 +67,13 @@ export const hookConfigGuard = z.object({
 
 export type HookConfig = z.infer<typeof hookConfigGuard>;
 
-/**
- * Management API hooks registration.
- *
- * Pre register the hooks that will be triggered by the management API.
- * Used by the managementApiHooks middleware.
- * Auto register the following hooks when the management API is called.
- */
 type ApiMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH';
 type ManagementApiHookKey = `${ApiMethod} ${string}`;
 
+/**
+ * Management API hooks registration.
+ * Define the hook event that should be triggered when the management API is called.
+ */
 export const managementApiHooksRegistration = Object.freeze({
   'POST /roles': 'Role.Created',
   'PATCH /roles/:id': 'Role.Updated',

packages/schemas/src/types/hook.ts

@@ -8,9 +8,10 @@ import type { userInfoSelectFields } from './user.js';
 // Define a without hookId type ahead.
 // That is because using Omit with Record<string, unknown> will loose type definition.
 // @see https://stackoverflow.com/questions/65013802/loose-type-definition-with-omit-and-keystring-unknown
-export type InteractionHookEventPayloadWithoutHookId = {
+export type InteractionHookEventPayload = {
   event: InteractionHookEvent;
   createdAt: string;
+  hookId: string;
   sessionId?: string;
   userAgent?: string;
   userId?: string;
@@ -19,9 +20,10 @@ export type InteractionHookEventPayloadWithoutHookId = {
   application?: Pick<Application, 'id' | 'type' | 'name' | 'description'>;
 } & Record<string, unknown>;
 
-export type DataHookEventPayloadWithoutHookId = {
+export type DataHookEventPayload = {
   event: DataHookEvent;
   createdAt: string;
+  hookId: string;
   ip?: string;
   userAgent?: string;
   body?: Record<string, unknown>;
@@ -30,18 +32,6 @@ export type DataHookEventPayloadWithoutHookId = {
   method?: string;
 } & Record<string, unknown>;
 
-export type InteractionHookEventPayload = InteractionHookEventPayloadWithoutHookId & {
-  hookId: string;
-};
-
-export type DataHookEventPayload = DataHookEventPayloadWithoutHookId & {
-  hookId: string;
-};
-
-export type HookEventPayloadWithoutHookId =
-  | InteractionHookEventPayloadWithoutHookId
-  | DataHookEventPayloadWithoutHookId;
-
 export type HookEventPayload = InteractionHookEventPayload | DataHookEventPayload;
 
 const hookExecutionStatsGuard = z.object({

packages/shared/src/utils/normalize-error.ts

@@ -66,7 +66,7 @@ function getCircularReplacer() {
 
 /**
  * Clone and stringify error object for logging purpose.
- * The stringified result  will be used as the error message.
+ * The stringified result will be used as the error message.
  * This is necessary because directly stringify an non-Error object will lose the stack trace.
  */
 export const normalizeError = (error: unknown) => {