Align the unreachable devfile exception message (#615)

Add a common exception message and use it for cases when: The devfile location is unavailable. The devfile content is is not valid: e.g the response content is an html content of an scm authorisation page.
eclipse-che · Nov 28, 2023 · 5baa3a7 · 5baa3a7
1 parent f34e804
commit 5baa3a7
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 10 deletions.
diff --git a/.ci/openshift-ci/common.sh b/.ci/openshift-ci/common.sh
@@ -101,6 +101,15 @@ testFactoryResolverNoPatOAuth() {
   echo "[INFO] Check factory resolver for public repository with NO PAT/OAuth setup"
   testFactoryResolverResponse $1 200
 
+  echo "[INFO] Check factory resolver for private repository with NO PAT/OAuth setup"
+  testFactoryResolverResponse $2 500
+}
+
+# check that raw devfile url factory resolver returns correct value without any PAT/OAuth setup
+testFactoryResolverNoPatOAuthRaw() {
+  echo "[INFO] Check factory resolver for public repository with NO PAT/OAuth setup"
+  testFactoryResolverResponse $1 200
+
   echo "[INFO] Check factory resolver for private repository with NO PAT/OAuth setup"
   testFactoryResolverResponse $2 400
 }

diff --git a/.ci/openshift-ci/test-azure-no-pat-oauth-flow.sh b/.ci/openshift-ci/test-azure-no-pat-oauth-flow.sh
@@ -26,8 +26,7 @@ source "${SCRIPT_DIR}"/common.sh
 trap "catchFinish" EXIT SIGINT
 
 setupTestEnvironment ${OCP_NON_ADMIN_USER_NAME}
-# due to the issue https://github.com/eclipse/che/issues/22469
-# testFactoryResolverNoPatOAuth ${PUBLIC_REPO_URL} ${PRIVATE_REPO_URL}
+testFactoryResolverNoPatOAuth ${PUBLIC_REPO_URL} ${PRIVATE_REPO_URL}
 testCloneGitRepoProjectShouldExists ${PUBLIC_REPO_WORKSPACE_NAME} ${PUBLIC_PROJECT_NAME} ${PUBLIC_REPO_URL} ${USER_CHE_NAMESPACE}
 deleteTestWorkspace ${PUBLIC_REPO_WORKSPACE_NAME} ${USER_CHE_NAMESPACE}
 testCloneGitRepoNoProjectExists ${PRIVATE_REPO_WORKSPACE_NAME} ${PRIVATE_PROJECT_NAME} ${PRIVATE_REPO_URL} ${USER_CHE_NAMESPACE}

diff --git a/.ci/openshift-ci/test-bitbucket-no-pat-oauth-flow-raw-devfile-url.sh b/.ci/openshift-ci/test-bitbucket-no-pat-oauth-flow-raw-devfile-url.sh
@@ -26,7 +26,7 @@ source "${SCRIPT_DIR}"/common.sh
 trap "catchFinish" EXIT SIGINT
 
 setupTestEnvironment ${OCP_NON_ADMIN_USER_NAME}
-testFactoryResolverNoPatOAuth ${PUBLIC_REPO_RAW_PATH_URL} ${PRIVATE_REPO_RAW_PATH_URL}
+testFactoryResolverNoPatOAuthRaw ${PUBLIC_REPO_RAW_PATH_URL} ${PRIVATE_REPO_RAW_PATH_URL}
 
 testCloneGitRepoProjectShouldExists ${PUBLIC_REPO_WORKSPACE_NAME} ${PUBLIC_PROJECT_NAME} ${PUBLIC_REPO_RAW_PATH_URL} ${USER_CHE_NAMESPACE}
 deleteTestWorkspace ${PUBLIC_REPO_WORKSPACE_NAME} ${USER_CHE_NAMESPACE}

diff --git a/.ci/openshift-ci/test-github-no-pat-oauth-flow-raw-devfile-url.sh b/.ci/openshift-ci/test-github-no-pat-oauth-flow-raw-devfile-url.sh
@@ -26,7 +26,7 @@ source "${SCRIPT_DIR}"/common.sh
 trap "catchFinish" EXIT SIGINT
 
 setupTestEnvironment ${OCP_NON_ADMIN_USER_NAME}
-testFactoryResolverNoPatOAuth ${PUBLIC_REPO_RAW_PATH_URL} ${PRIVATE_REPO_RAW_PATH_URL}
+testFactoryResolverNoPatOAuthRaw ${PUBLIC_REPO_RAW_PATH_URL} ${PRIVATE_REPO_RAW_PATH_URL}
 
 testCloneGitRepoNoProjectExists ${PUBLIC_REPO_WORKSPACE_NAME} ${PUBLIC_PROJECT_NAME} ${PUBLIC_REPO_RAW_PATH_URL} ${USER_CHE_NAMESPACE}
 deleteTestWorkspace ${PUBLIC_REPO_WORKSPACE_NAME} ${USER_CHE_NAMESPACE}

diff --git a/.ci/openshift-ci/test-gitlab-no-pat-oauth-flow-raw-devfile-url.sh b/.ci/openshift-ci/test-gitlab-no-pat-oauth-flow-raw-devfile-url.sh
@@ -26,7 +26,7 @@ source "${SCRIPT_DIR}"/common.sh
 trap "catchFinish" EXIT SIGINT
 
 setupTestEnvironment ${OCP_NON_ADMIN_USER_NAME}
-testFactoryResolverNoPatOAuth ${PUBLIC_REPO_RAW_PATH_URL} ${PRIVATE_REPO_RAW_PATH_URL}
+testFactoryResolverNoPatOAuthRaw ${PUBLIC_REPO_RAW_PATH_URL} ${PRIVATE_REPO_RAW_PATH_URL}
 
 testCloneGitRepoNoProjectExists ${PUBLIC_REPO_WORKSPACE_NAME} ${PUBLIC_PROJECT_NAME} ${PUBLIC_REPO_RAW_PATH_URL} ${USER_CHE_NAMESPACE}
 deleteTestWorkspace ${PUBLIC_REPO_WORKSPACE_NAME} ${USER_CHE_NAMESPACE}

diff --git a/.../src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java b/.../src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java
@@ -13,6 +13,7 @@
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher.OAUTH_2_PREFIX;
+import static org.eclipse.che.api.factory.server.scm.exception.ExceptionMessages.getDevfileConnectionErrorMessage;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -123,7 +124,7 @@ protected String fetchContentWithoutToken(String requestURL)
         }
       }
       throw new DevfileException(
-          "Could not reach devfile at " + "`" + exception.getMessage() + "`", exception);
+          getDevfileConnectionErrorMessage(exception.getMessage()), exception);
     }
   }
 

diff --git a/...ory/src/main/java/org/eclipse/che/api/factory/server/scm/exception/ExceptionMessages.java b/...ory/src/main/java/org/eclipse/che/api/factory/server/scm/exception/ExceptionMessages.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) 2012-2023 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.api.factory.server.scm.exception;
+
+public class ExceptionMessages {
+  public static String getDevfileConnectionErrorMessage(String location) {
+    return String.format("Could not reach devfile at %s", location);
+  }
+}
diff --git a/...actory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java b/...actory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java
@@ -13,6 +13,7 @@
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.eclipse.che.api.factory.server.ApiExceptionMapper.toApiException;
+import static org.eclipse.che.api.factory.server.scm.exception.ExceptionMessages.getDevfileConnectionErrorMessage;
 import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
 import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION;
 import static org.eclipse.che.api.workspace.shared.Constants.WORKSPACE_TOOLING_EDITOR_ATTRIBUTE;
@@ -130,7 +131,7 @@ public Optional<FactoryMetaDto> createFactoryFromDevfile(
         continue;
       } catch (DevfileException e) {
         LOG.debug("Unexpected devfile exception: {}", e.getMessage());
-        throw toApiException(e, location);
+        throw new ApiException(e.getMessage());
       }
       if (isNullOrEmpty(devfileYamlContent)) {
         return Optional.empty();
@@ -142,7 +143,7 @@ public Optional<FactoryMetaDto> createFactoryFromDevfile(
         try {
           devfileVersionDetector.devfileVersion(parsedDevfile);
         } catch (DevfileException e) {
-          throw new ApiException("Failed to fetch devfile");
+          throw new ApiException(getDevfileConnectionErrorMessage(devfileLocation));
         }
         return Optional.of(
             createFactory(parsedDevfile, overrideProperties, fileContentProvider, location));

diff --git a/...ry/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java b/...ry/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java
@@ -441,7 +441,8 @@ public String location() {
                   }
                 }));
 
-    when(fileContentProvider.fetchContent(anyString())).thenThrow(new DevfileException("", cause));
+    when(fileContentProvider.fetchContent(anyString()))
+        .thenThrow(new DevfileException(expectedMessage, cause));
 
     // when
     try {
@@ -457,7 +458,7 @@ public String location() {
 
   @Test(
       expectedExceptions = ApiException.class,
-      expectedExceptionsMessageRegExp = "Failed to fetch devfile")
+      expectedExceptionsMessageRegExp = "Could not reach devfile at location")
   public void shouldThrowErrorOnUnsupportedDevfileContent()
       throws ApiException, DevfileException, IOException {
     JsonNode jsonNode = mock(JsonNode.class);