fix: Improve error handling in case of failure when retrieving API doc (

#3932) * propagate exception to UI Signed-off-by: Andrea Tabone <[email protected]> * refactoring Signed-off-by: Andrea Tabone <[email protected]> * fix tests Signed-off-by: Andrea Tabone <[email protected]> * fix missing props validation Signed-off-by: Andrea Tabone <[email protected]> * address comments Signed-off-by: Andrea Tabone <[email protected]> --------- Signed-off-by: Andrea Tabone <[email protected]>
zowe · Dec 16, 2024 · 3fb0d59 · 3fb0d59
1 parent 1ce5918
commit 3fb0d59
Show file tree

Hide file tree

Showing 10 changed files with 92 additions and 50 deletions.
diff --git a/...ervices/src/main/java/org/zowe/apiml/apicatalog/controllers/api/ApiCatalogController.java b/...ervices/src/main/java/org/zowe/apiml/apicatalog/controllers/api/ApiCatalogController.java
@@ -172,9 +172,8 @@ private void setApiDocToService(APIContainer apiContainer) {
                 String defaultApiVersion = cachedApiDocService.getDefaultApiVersionForService(serviceId);
                 apiService.setDefaultApiVersion(defaultApiVersion);
             } catch (Exception e) {
-                log.debug("An error occurred when trying to fetch ApiDoc for service: " + serviceId +
-                    ", processing can continue but this service will not be able to display any Api Documentation.\n" +
-                    "Error Message: " + e.getMessage());
+                log.debug("An error occurred when trying to fetch ApiDoc for service: {}, processing can continue but this service will not be able to display any Api Documentation.\nError:", serviceId, e);
+                apiService.setApiDocErrorMessage("Failed to fetch API documentation: " + e.getMessage());
             }
         });
     }

diff --git a/api-catalog-services/src/main/java/org/zowe/apiml/apicatalog/model/APIService.java b/api-catalog-services/src/main/java/org/zowe/apiml/apicatalog/model/APIService.java
@@ -57,6 +57,9 @@ public class APIService implements Serializable {
     @Schema(description = "The default API version for this service")
     private String defaultApiVersion = "v1";
 
+    @Schema(description = "The error message occurred when trying to retrieve the API doc")
+    private String apiDocErrorMessage;
+
     @Schema(description = "The available API versions for this service")
     private List<String> apiVersions;
 

diff --git a/...services/src/main/java/org/zowe/apiml/apicatalog/services/cached/CachedApiDocService.java b/...services/src/main/java/org/zowe/apiml/apicatalog/services/cached/CachedApiDocService.java
@@ -22,6 +22,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
 import java.util.function.UnaryOperator;
 
 /**
@@ -48,34 +49,65 @@ public CachedApiDocService(APIDocRetrievalService apiDocRetrievalService, Transf
     }
 
     /**
-     * Update the api docs for this service
+     * Fetches API documentation for a given service and version.
      *
-     * @param serviceId  service identifier
-     * @param apiVersion the version of the API
-     * @return api doc info for the requested service id
+     * @param serviceId    service identifier
+     * @param cacheKeySuffix version or default API key
+     * @param retrievalLogic supplier providing the API documentation retrieval logic
+     * @return the API documentation
      */
-    public String getApiDocForService(final String serviceId, final String apiVersion) {
-        // First try to fetch apiDoc from the DS
+    private String fetchApiDoc(
+        final String serviceId,
+        final String cacheKeySuffix,
+        final Supplier<ApiDocInfo> retrievalLogic
+    ) {
+        ApiDocCacheKey cacheKey = new ApiDocCacheKey(serviceId, cacheKeySuffix);
+        String errorMessage = "";
+        Exception exception = null;
+
+        // Try to fetch API doc from the retrieval logic
         try {
-            ApiDocInfo apiDocInfo = apiDocRetrievalService.retrieveApiDoc(serviceId, apiVersion);
+            ApiDocInfo apiDocInfo = retrievalLogic.get();
             if (apiDocInfo != null && apiDocInfo.getApiDocContent() != null) {
                 String apiDoc = transformApiDocService.transformApiDoc(serviceId, apiDocInfo);
-                CachedApiDocService.serviceApiDocs.put(new ApiDocCacheKey(serviceId, apiVersion), apiDoc);
+                CachedApiDocService.serviceApiDocs.put(cacheKey, apiDoc);
                 return apiDoc;
+            } else {
+                log.debug("No API Documentation found for the service {}", serviceId);
             }
         } catch (Exception e) {
-            log.debug("Exception updating API doc in cache for '{} {}'", serviceId, apiVersion, e);
+            log.debug("Exception updating API doc in cache for '{} {}'", serviceId, cacheKeySuffix, e);
+            errorMessage = e.getMessage();
+            exception = e;
         }
 
-        // if no DS is available try to use cached data
-        String apiDoc = CachedApiDocService.serviceApiDocs.get(new ApiDocCacheKey(serviceId, apiVersion));
+        // If no DS is available, try to use cached data
+        String apiDoc = CachedApiDocService.serviceApiDocs.get(cacheKey);
         if (apiDoc != null) {
+            log.debug("Using cached API doc for service '{}'", serviceId);
             return apiDoc;
         }
 
-        // cannot obtain apiDoc ends with exception
-        log.error("No API doc available for '{} {}'", serviceId, apiVersion);
-        throw new ApiDocNotFoundException(exceptionMessage.apply(serviceId));
+        // Cannot obtain API doc, throw exception
+        log.error("No API doc available for '{} {}'", serviceId, cacheKeySuffix);
+        throw new ApiDocNotFoundException(
+            exceptionMessage.apply(serviceId) + " Root cause: " + errorMessage, exception
+        );
+    }
+
+    /**
+     * Update the api docs for this service
+     *
+     * @param serviceId  service identifier
+     * @param apiVersion the version of the API
+     * @return api doc info for the requested service id
+     */
+    public String getApiDocForService(final String serviceId, final String apiVersion) {
+        return fetchApiDoc(
+            serviceId,
+            apiVersion,
+            () -> apiDocRetrievalService.retrieveApiDoc(serviceId, apiVersion)
+        );
     }
 
     /**
@@ -97,27 +129,11 @@ public void updateApiDocForService(final String serviceId, final String apiVersi
      * @return api doc info for the latest API of the request service id
      */
     public String getDefaultApiDocForService(final String serviceId) {
-        // First try to fetch apiDoc from the DS
-        try {
-            ApiDocInfo apiDocInfo = apiDocRetrievalService.retrieveDefaultApiDoc(serviceId);
-            if (apiDocInfo != null && apiDocInfo.getApiDocContent() != null) {
-                String apiDoc = transformApiDocService.transformApiDoc(serviceId, apiDocInfo);
-                CachedApiDocService.serviceApiDocs.put(new ApiDocCacheKey(serviceId, DEFAULT_API_KEY), apiDoc);
-                return apiDoc;
-            }
-        } catch (Throwable t) {
-            log.debug("Exception updating default API doc in cache for '{}'.", serviceId, t);
-        }
-
-        // if no DS is available try to use cached data
-        String apiDoc = CachedApiDocService.serviceApiDocs.get(new ApiDocCacheKey(serviceId, DEFAULT_API_KEY));
-        if (apiDoc != null) {
-            return apiDoc;
-        }
-
-        // cannot obtain apiDoc ends with exception
-        log.error("No default API doc available for service '{}'", serviceId);
-        throw new ApiDocNotFoundException(exceptionMessage.apply(serviceId));
+        return fetchApiDoc(
+            serviceId,
+            DEFAULT_API_KEY,
+            () -> apiDocRetrievalService.retrieveDefaultApiDoc(serviceId)
+        );
     }
 
     /**
@@ -155,7 +171,7 @@ public List<String> getApiVersionsForService(final String serviceId) {
             return versions;
         }
 
-        // cannot obtain apiDoc ends with exception
+        // Cannot obtain API doc, end with exception
         log.error("No API versions available for service '{}'", serviceId);
         throw new ApiVersionNotFoundException("No API versions were retrieved for the service " + serviceId + ".");
     }

diff --git a/...catalog-services/src/main/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV2Service.java b/...catalog-services/src/main/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV2Service.java
@@ -40,11 +40,10 @@ public ApiDocV2Service(GatewayClient gatewayClient) {
     }
 
     public String transformApiDoc(String serviceId, ApiDocInfo apiDocInfo) {
-
         Swagger swagger = new SwaggerParser().readWithInfo(apiDocInfo.getApiDocContent()).getSwagger();
         if (swagger == null) {
             log.debug("Could not convert response body to a Swagger object.");
-            throw new UnexpectedTypeException("Response is not a Swagger type object.");
+            throw new UnexpectedTypeException(String.format("The Swagger definition for service '%s' was retrieved but was not a valid JSON document.", serviceId));
         }
 
         boolean hidden = swagger.getTag(HIDDEN_TAG) != null;

diff --git a/...catalog-services/src/main/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV3Service.java b/...catalog-services/src/main/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV3Service.java
@@ -64,7 +64,8 @@ public String transformApiDoc(String serviceId, ApiDocInfo apiDocInfo) {
             if (parseResult.getMessages() == null) {
                 throw new UnexpectedTypeException("Response is not an OpenAPI type object.");
             } else {
-                throw new UnexpectedTypeException(parseResult.getMessages().toString());
+                throw new UnexpectedTypeException(
+                    String.format("The OpenAPI for service '%s' was retrieved but was not a valid JSON document. '%s'", serviceId, parseResult.getMessages().toString()));
             }
         }
 

diff --git a/...ices/src/test/java/org/zowe/apiml/apicatalog/services/cached/CachedApiDocServiceTest.java b/...ices/src/test/java/org/zowe/apiml/apicatalog/services/cached/CachedApiDocServiceTest.java
@@ -116,7 +116,7 @@ void givenInvalidApiDoc_whenRetrieving_thenThrowException() {
         Exception exception = assertThrows(ApiDocNotFoundException.class,
             () -> cachedApiDocService.getApiDocForService(serviceId, version),
             "Expected exception is not ApiDocNotFoundException");
-        assertEquals("No API Documentation was retrieved for the service Service.", exception.getMessage());
+        assertEquals("No API Documentation was retrieved for the service Service. Root cause: ", exception.getMessage());
     }
 
     @Nested
@@ -220,7 +220,7 @@ void givenInvalidApiDoc_whenRetrievingDefault_thenThrowException() {
         Exception exception = assertThrows(ApiDocNotFoundException.class,
             () -> cachedApiDocService.getDefaultApiDocForService(serviceId),
             "Expected exception is not ApiDocNotFoundException");
-        assertEquals("No API Documentation was retrieved for the service service.", exception.getMessage());
+        assertEquals("No API Documentation was retrieved for the service service. Root cause: ", exception.getMessage());
     }
 
     @Nested

diff --git a/...log-services/src/test/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV2ServiceTest.java b/...log-services/src/test/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV2ServiceTest.java
@@ -80,7 +80,7 @@ void givenSwaggerJsonNotAsExpectedFormat_whenConvertToSwagger_thenThrowIOExcepti
         ApiDocInfo apiDocInfo = new ApiDocInfo(null, apiDocContent, null);
 
         Exception exception = assertThrows(UnexpectedTypeException.class, () -> apiDocV2Service.transformApiDoc(SERVICE_ID, apiDocInfo));
-        assertEquals("Response is not a Swagger type object.", exception.getMessage());
+        assertEquals("The Swagger definition for service 'serviceId' was retrieved but was not a valid JSON document.", exception.getMessage());
     }
 
     @Nested

diff --git a/...log-services/src/test/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV3ServiceTest.java b/...log-services/src/test/java/org/zowe/apiml/apicatalog/swagger/api/ApiDocV3ServiceTest.java
@@ -179,14 +179,14 @@ void givenEmptyJson() {
                 ApiDocInfo apiDocInfo = new ApiDocInfo(apiInfo, invalidJson, null);
 
                 Exception exception = assertThrows(UnexpectedTypeException.class, () -> apiDocV3Service.transformApiDoc(SERVICE_ID, apiDocInfo));
-                assertEquals("[Null or empty definition]", exception.getMessage());
+                assertEquals("The OpenAPI for service 'serviceId' was retrieved but was not a valid JSON document. '[Null or empty definition]'", exception.getMessage());
             }
 
             @Test
             void givenInvalidJson() {
                 String invalidJson = "nonsense";
-                String error = "[Cannot construct instance of `java.util.LinkedHashMap` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('nonsense')\n" +
-                    " at [Source: UNKNOWN; byte offset: #UNKNOWN]]";
+                String error = "The OpenAPI for service 'serviceId' was retrieved but was not a valid JSON document. '[Cannot construct instance of `java.util.LinkedHashMap` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('nonsense')\n" +
+                    " at [Source: UNKNOWN; byte offset: #UNKNOWN]]'";
                 ApiInfo apiInfo = new ApiInfo(API_ID, "api/v1", API_VERSION, "https://localhost:10014/apicatalog/api-doc",null, "https://www.zowe.org");
                 ApiDocInfo apiDocInfo = new ApiDocInfo(apiInfo, invalidJson, null);
 

diff --git a/api-catalog-ui/frontend/src/components/Swagger/SwaggerUIApiml.jsx b/api-catalog-ui/frontend/src/components/Swagger/SwaggerUIApiml.jsx
@@ -194,8 +194,9 @@ export default class SwaggerUIApiml extends Component {
                 {error && (
                     <div style={{ width: '100%', background: '#ffffff', paddingLeft: 55 }}>
                         <h4 id="no-doc_message">
-                            API documentation could not be retrieved. There may be something wrong in your Swagger
-                            definition. Please review the values of 'schemes', 'host' and 'basePath'.
+                            {selectedService.apiDocErrorMessage
+                                ? selectedService.apiDocErrorMessage
+                                : "API documentation could not be retrieved. There may be something wrong in your Swagger definition. Please review the values of 'schemes', 'host' and 'basePath'."}
                         </h4>
                     </div>
                 )}
@@ -212,6 +213,7 @@ export default class SwaggerUIApiml extends Component {
 SwaggerUIApiml.propTypes = {
     selectedService: PropTypes.shape({
         apiDoc: PropTypes.string,
+        apiDocErrorMessage: PropTypes.string,
     }).isRequired,
     url: PropTypes.string,
 };

diff --git a/api-catalog-ui/frontend/src/components/Swagger/SwaggerUIApiml.test.jsx b/api-catalog-ui/frontend/src/components/Swagger/SwaggerUIApiml.test.jsx
@@ -109,6 +109,28 @@ describe('>>> Swagger component tests', () => {
         expect(container.textContent).toContain(`API documentation could not be retrieved`);
     });
 
+    it('should not render swagger if apiDoc contains error', async () => {
+        const service = {
+            serviceId: 'testservice',
+            title: 'Spring Boot Enabler Service',
+            description: 'Dummy Service for enabling others',
+            status: 'UP',
+            secured: false,
+            homePageUrl: 'http://localhost:10013/enabler/',
+            basePath: '/enabler/api/v1',
+            defaultApiVersion: 0,
+            apiDoc: null,
+            apiDocErrorMessage: "API documentation could not be retrieved. Invalid JSON"
+
+        };
+
+        const container = document.createElement('div');
+        document.body.appendChild(container);
+        const root = createRoot(container);
+        await act(async () => root.render(<SwaggerUI selectedService={service} />));
+        expect(container.textContent).toEqual(`API documentation could not be retrieved. Invalid JSON`);
+    });
+
     it('should transform swagger server url', async () => {
         const endpoint = '/enabler/api/v1';
         const service = {