From b88df39ba1ed161a50bd2fb44d77417c384baad6 Mon Sep 17 00:00:00 2001
From: Johan Preynat <johan.preynat@gmail.com>
Date: Fri, 22 Nov 2024 07:24:33 +0100
Subject: [PATCH] Throw 400 exposable errors instead of internal errors when
 Git sync config value are missing/incomplete (#632)

---
 integrations/github/src/provider.ts | 12 ++++++------
 integrations/github/src/sync.ts     |  4 ++--
 integrations/github/src/utils.ts    | 14 ++++++++++----
 integrations/gitlab/src/provider.ts |  8 ++++----
 integrations/gitlab/src/sync.ts     |  6 +++---
 integrations/gitlab/src/utils.ts    | 12 ++++++++++--
 6 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/integrations/github/src/provider.ts b/integrations/github/src/provider.ts
index abe9aa5ce..12550de82 100644
--- a/integrations/github/src/provider.ts
+++ b/integrations/github/src/provider.ts
@@ -39,8 +39,8 @@ export async function getGitHubAppJWT(context: GithubRuntimeContext): Promise<st
  * Returns the URL of the Git repository.
  */
 export function getRepositoryUrl(config: GitHubSpaceConfiguration, withExtension = false): string {
-    assertIsDefined(config.accountName, { label: 'config.accountName' });
-    assertIsDefined(config.repoName, { label: 'config.repoName' });
+    assertIsDefined(config.accountName, { label: 'config.accountName', statusCode: 400 });
+    assertIsDefined(config.repoName, { label: 'config.repoName', statusCode: 400 });
 
     return `https://github.com/${config.accountName}/${config.repoName}${
         withExtension ? '.git' : ''
@@ -54,7 +54,7 @@ export async function getRepositoryAuth(
     context: GithubRuntimeContext,
     config: GitHubSpaceConfiguration,
 ) {
-    assertIsDefined(config.installation, { label: 'config.installation' });
+    assertIsDefined(config.installation, { label: 'config.installation', statusCode: 400 });
 
     const appJWT = await getGitHubAppJWT(context);
     const installationAccessToken = await createAppInstallationAccessToken(
@@ -84,9 +84,9 @@ export async function updateCommitStatus(
         description: string;
     },
 ) {
-    assertIsDefined(config.accountName, { label: 'config.accountName' });
-    assertIsDefined(config.repoName, { label: 'config.repoName' });
-    assertIsDefined(config.installation, { label: 'config.installation' });
+    assertIsDefined(config.accountName, { label: 'config.accountName', statusCode: 400 });
+    assertIsDefined(config.repoName, { label: 'config.repoName', statusCode: 400 });
+    assertIsDefined(config.installation, { label: 'config.installation', statusCode: 400 });
 
     const appJWT = await getGitHubAppJWT(context);
     const installationAccessToken = await createAppInstallationAccessToken(
diff --git a/integrations/github/src/sync.ts b/integrations/github/src/sync.ts
index f47c33f2c..55bc4a033 100644
--- a/integrations/github/src/sync.ts
+++ b/integrations/github/src/sync.ts
@@ -68,7 +68,7 @@ export async function triggerImport(
         return;
     }
 
-    assertIsDefined(config.branch, { label: 'config.branch' });
+    assertIsDefined(config.branch, { label: 'config.branch', statusCode: 400 });
 
     logger.info(`Initiating an import from GitHub to GitBook space ${spaceId}`);
 
@@ -130,7 +130,7 @@ export async function triggerExport(
         return;
     }
 
-    assertIsDefined(config.branch, { label: 'config.branch' });
+    assertIsDefined(config.branch, { label: 'config.branch', statusCode: 400 });
 
     logger.info(`Initiating an export from space ${spaceId} to GitHub`);
 
diff --git a/integrations/github/src/utils.ts b/integrations/github/src/utils.ts
index cad7fc7cc..ecf3b427d 100644
--- a/integrations/github/src/utils.ts
+++ b/integrations/github/src/utils.ts
@@ -1,6 +1,7 @@
 import { GitSyncOperationState, IntegrationSpaceInstallation } from '@gitbook/api';
 
 import type { GitHubSpaceConfiguration } from './types';
+import { ExposableError } from '@gitbook/runtime';
 
 export const BRANCH_REF_PREFIX = 'refs/heads/';
 
@@ -75,12 +76,17 @@ export function computeConfigQueryKey(
 
 export function assertIsDefined<T>(
     value: T,
-    options: {
-        label: string;
-    },
+    options: { label: string; statusCode?: number },
 ): asserts value is NonNullable<T> {
+    const { label, statusCode = 500 } = options;
+
     if (value === undefined || value === null) {
-        throw new Error(`Expected value (${options.label}) to be defined, but received ${value}`);
+        const errorMsg = `Expected value (${label}) to be defined, but received ${value}`;
+        if (statusCode >= 400 && statusCode < 500) {
+            throw new ExposableError(errorMsg, statusCode);
+        }
+
+        throw new Error(errorMsg);
     }
 }
 
diff --git a/integrations/gitlab/src/provider.ts b/integrations/gitlab/src/provider.ts
index 4ba19311b..9d898685c 100644
--- a/integrations/gitlab/src/provider.ts
+++ b/integrations/gitlab/src/provider.ts
@@ -23,7 +23,7 @@ export async function installWebhook(
 ) {
     const config = getSpaceConfigOrThrow(spaceInstallation);
 
-    assertIsDefined(config.project, { label: 'config.project' });
+    assertIsDefined(config.project, { label: 'config.project', statusCode: 400 });
 
     const projectId = config.project;
     const id = await addProjectWebhook(config, config.project, webhookUrl, webhookToken);
@@ -38,8 +38,8 @@ export async function installWebhook(
  * project.
  */
 export async function uninstallWebhook(config: GitLabSpaceConfiguration) {
-    assertIsDefined(config.project, { label: 'config.project' });
-    assertIsDefined(config.webhookId, { label: 'config.webhookId' });
+    assertIsDefined(config.project, { label: 'config.project', statusCode: 400 });
+    assertIsDefined(config.webhookId, { label: 'config.webhookId', statusCode: 400 });
 
     const projectId = config.project;
     const webhookId = config.webhookId;
@@ -62,7 +62,7 @@ export async function updateCommitStatus(
         description: string;
     },
 ) {
-    assertIsDefined(config.project, { label: 'config.project' });
+    assertIsDefined(config.project, { label: 'config.project', statusCode: 400 });
 
     const projectId = config.project;
 
diff --git a/integrations/gitlab/src/sync.ts b/integrations/gitlab/src/sync.ts
index 5e98105e6..26b13d8c4 100644
--- a/integrations/gitlab/src/sync.ts
+++ b/integrations/gitlab/src/sync.ts
@@ -4,7 +4,7 @@ import {
     IntegrationSpaceInstallation,
     Revision,
 } from '@gitbook/api';
-import { Logger } from '@gitbook/runtime';
+import { ExposableError, Logger } from '@gitbook/runtime';
 
 import {
     getGitCommitURL,
@@ -68,7 +68,7 @@ export async function triggerImport(
         return;
     }
 
-    assertIsDefined(config.branch, { label: 'config.branch' });
+    assertIsDefined(config.branch, { label: 'config.branch', statusCode: 400 });
 
     logger.info(`Initiating an import from GitLab to GitBook space ${spaceId}`);
 
@@ -130,7 +130,7 @@ export async function triggerExport(
         return;
     }
 
-    assertIsDefined(config.branch, { label: 'config.branch' });
+    assertIsDefined(config.branch, { label: 'config.branch', statusCode: 400 });
 
     logger.info(`Initiating an export from space ${spaceId} to GitLab`);
 
diff --git a/integrations/gitlab/src/utils.ts b/integrations/gitlab/src/utils.ts
index 60ae9b0a1..c895753a8 100644
--- a/integrations/gitlab/src/utils.ts
+++ b/integrations/gitlab/src/utils.ts
@@ -1,6 +1,7 @@
 import type { GitSyncOperationState, IntegrationSpaceInstallation } from '@gitbook/api';
 
 import type { GitLabSpaceConfiguration } from './types';
+import { ExposableError } from '@gitbook/runtime';
 
 export const BRANCH_REF_PREFIX = 'refs/heads/';
 
@@ -100,10 +101,17 @@ export async function verifySignature(
 
 export function assertIsDefined<T>(
     value: T,
-    options: { label: string },
+    options: { label: string; statusCode?: number },
 ): asserts value is NonNullable<T> {
+    const { label, statusCode = 500 } = options;
+
     if (value === undefined || value === null) {
-        throw new Error(`Expected value (${options.label}) to be defined, but received ${value}`);
+        const errorMsg = `Expected value (${label}) to be defined, but received ${value}`;
+        if (statusCode >= 400 && statusCode < 500) {
+            throw new ExposableError(errorMsg, statusCode);
+        }
+
+        throw new Error(errorMsg);
     }
 }