Improve OpenAPI parsing errors (#3555)

.changeset/tough-beers-own.md

@@ -0,0 +1,6 @@
+---
+"@gitbook/openapi-parser": patch
+"gitbook": patch
+---
+
+Improve OpenAPI parsing errors

packages/gitbook/src/lib/openapi/fetch.ts

@@ -36,25 +36,28 @@ export async function fetchOpenAPIFilesystem(
         return { filesystem: null, specUrl: null };
     }
 
-    const filesystem = await (() => {
+    const result = await (() => {
+        // If the reference is a new OpenAPI reference, we return it.
         if (ref.kind === 'openapi') {
             assert(resolved.openAPIFilesystem);
             return resolved.openAPIFilesystem;
         }
+        // For legacy blocks ("swagger"), we need to fetch the file system.
         return fetchFilesystem(resolved.href, context.space.id);
     })();
 
-    if ('error' in filesystem) {
+    if ('error' in result) {
-        throw new OpenAPIParseError(filesystem.error.message, { code: filesystem.error.code });
+        throw new OpenAPIParseError(result.error.message, { code: result.error.code });
     }
 
-    return {
+    return { filesystem: result, specUrl: resolved.href };
-        filesystem,
-        specUrl: resolved.href,
-    };
 }
 
-const fetchFilesystem = async (
+/**
+ * Fetch the filesystem from the URL.
+ * It's used for legacy "swagger" blocks.
+ */
+async function fetchFilesystem(
     url: string,
     spaceId: string
 ): Promise<
@@ -65,11 +68,11 @@ const fetchFilesystem = async (
               message: string;
           };
       }
-> => {
+> {
     'use cache';
     try {
         cacheTag(getCacheTag({ tag: 'space', space: spaceId }));
-        return await fetchFilesystemUncached(url);
+        return await fetchFilesystemNoCache(url);
     } catch (error) {
         // To avoid hammering the file with requests, we cache the error for around a minute.
         cacheLife('minutes');
@@ -86,21 +89,16 @@ const fetchFilesystem = async (
         console.error('Unknown error while fetching OpenAPI file:', error);
         return { error: { code: 'invalid' as const, message: 'Unknown error' } };
     }
-};
+}
 
-async function fetchFilesystemUncached(
+async function fetchFilesystemNoCache(url: string) {
-    url: string,
+    console.log(url);
-    options?: {
-        signal?: AbortSignal;
-    }
-) {
     // Wrap the raw string to prevent invalid URLs from being passed to fetch.
     // This can happen if the URL has whitespace, which is currently handled differently by Cloudflare's implementation of fetch:
     // https://github.com/cloudflare/workerd/issues/1957
     const response = await fetch(new URL(url), {
         ...noCacheFetchOptions,
         cache: 'no-store',
-        signal: options?.signal,
     });
 
     if (!response.ok) {

packages/gitbook/src/lib/openapi/resolveOpenAPIOperationBlock.ts

@@ -9,10 +9,7 @@ import type {
 
 type ResolveOpenAPIOperationBlockResult = ResolveOpenAPIBlockResult<OpenAPIOperationData>;
 
-const weakmap = new WeakMap<
+const cache = new WeakMap<AnyOpenAPIOperationsBlock, Promise<ResolveOpenAPIOperationBlockResult>>();
-    AnyOpenAPIOperationsBlock,
-    Promise<ResolveOpenAPIOperationBlockResult>
->();
 
 /**
  * Cache the result of resolving an OpenAPI block.
@@ -21,22 +18,24 @@ const weakmap = new WeakMap<
 export function resolveOpenAPIOperationBlock(
     args: ResolveOpenAPIBlockArgs<AnyOpenAPIOperationsBlock>
 ): Promise<ResolveOpenAPIOperationBlockResult> {
-    if (weakmap.has(args.block)) {
+    const inCache = cache.get(args.block);
-        return weakmap.get(args.block)!;
+    if (inCache) {
+        return inCache;
     }
 
-    const result = baseResolveOpenAPIOperationBlock(args);
+    const promise = resolveOpenAPIOperationBlockNoCache(args);
-    weakmap.set(args.block, result);
+    cache.set(args.block, promise);
-    return result;
+    return promise;
 }
 
 /**
  * Resolve OpenAPI operation block.
  */
-async function baseResolveOpenAPIOperationBlock(
+async function resolveOpenAPIOperationBlockNoCache(
     args: ResolveOpenAPIBlockArgs<AnyOpenAPIOperationsBlock>
 ): Promise<ResolveOpenAPIOperationBlockResult> {
     const { context, block } = args;
+
     if (!block.data.path || !block.data.method) {
         return { data: null, specUrl: null };
     }

packages/openapi-parser/src/parse.test.ts

@@ -39,7 +39,9 @@ describe('#parseOpenAPI', () => {
             });
         } catch (error) {
             if (error instanceof OpenAPIParseError) {
-                expect(error.message).toContain('Invalid OpenAPI document');
+                expect(error.message).toContain(
+                    'Can’t find supported Swagger/OpenAPI version in the provided document, version must be a string.'
+                );
             }
         }
     });

packages/openapi-parser/src/v3.ts

@@ -12,6 +12,19 @@ export async function parseOpenAPIV3(input: ParseOpenAPIInput): Promise<ParseOpe
     const { value, rootURL, options = {} } = input;
     const result = await validate(value);
 
+    // If there is no version, we consider it invalid instantely.
+    if (!result.version) {
+        throw new OpenAPIParseError(
+            'Can’t find supported Swagger/OpenAPI version in the provided document, version must be a string.',
+            {
+                code: 'invalid',
+                rootURL,
+                errors: result.errors,
+            }
+        );
+    }
+
+    // If the version is 2.0, we throw an error to trigger the upgrade.
     if (result.version === '2.0') {
         throw new OpenAPIParseError('Only OpenAPI v3 is supported', {
             code: 'parse-v2-in-v3',
@@ -21,7 +34,7 @@ export async function parseOpenAPIV3(input: ParseOpenAPIInput): Promise<ParseOpe
 
     // We don't rely on `result.invalid` because it's too strict.
     // If we succeed in parsing a schema, then we consider it valid.
-    if (!result.specification || !result.version) {
+    if (!result.specification) {
         throw new OpenAPIParseError('Invalid OpenAPI document', {
             code: 'invalid',
             rootURL,