Merge pull request #3269 from mapfish/better-errors-master-GSSGA-96

Better errors messages
mapfish · May 21, 2024 · c642b13 · c642b13
2 parents 97fee55 + 8c0535e
commit c642b13
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 23 deletions.
diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
@@ -30,6 +30,27 @@ jobs:
         with:
           fetch-depth: 0
 
+      - run: echo "${HOME}/.local/bin" >> ${GITHUB_PATH}
+      - run: python3 -m pip install --user --requirement=ci/requirements.txt
+
+      - uses: actions/cache@v4
+        with:
+          path: ~/.cache/pre-commit
+          key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
+          restore-keys: "pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}\npre-commit-"
+      - run: pre-commit run --all-files
+      - run: git diff --exit-code --patch > /tmp/pre-commit.patch || true
+        if: failure()
+      - uses: actions/upload-artifact@v4
+        with:
+          name: Apply pre-commit fix.patch
+          path: /tmp/pre-commit.patch
+          retention-days: 1
+        if: failure()
+
+      - name: Print environment information
+        run: c2cciutils-env
+
       - name: Get tag
         id: tag2
         uses: frabert/[email protected]
@@ -46,6 +67,7 @@ jobs:
           github-gopass-ci-token: ${{secrets.GOPASS_CI_GITHUB_TOKEN}}
           patterns: docker
         if: env.HAS_SECRETS == 'HAS_SECRETS'
+
       - run: gpg --export-secret-keys --armor D121AF2DFA8E140688BD968930C9B913FD42EF13 > CI.asc
         if: env.HAS_SECRETS == 'HAS_SECRETS'
 
@@ -77,26 +99,6 @@ jobs:
         if: env.HAS_SECRETS == 'HAS_SECRETS'
       - run: git diff
 
-      - run: echo "${HOME}/.local/bin" >> ${GITHUB_PATH}
-      - run: python3 -m pip install --user --requirement=ci/requirements.txt
-
-      - uses: actions/cache@v4
-        with:
-          path: ~/.cache/pre-commit
-          key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
-          restore-keys: "pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}\npre-commit-"
-      - run: pre-commit run --all-files
-      - run: git diff --exit-code --patch > /tmp/pre-commit.patch || true
-        if: failure()
-      - uses: actions/upload-artifact@v4
-        with:
-          name: Apply pre-commit fix.patch
-          path: /tmp/pre-commit.patch
-          retention-days: 1
-        if: failure()
-      - name: Print environment information
-        run: c2cciutils-env
-
       - run: make build
 
       - run: make checks

diff --git a/core/src/main/java/org/mapfish/print/http/ErrorResponseClientHttpResponse.java b/core/src/main/java/org/mapfish/print/http/ErrorResponseClientHttpResponse.java
@@ -9,6 +9,9 @@
 public class ErrorResponseClientHttpResponse extends AbstractClientHttpResponse {
   private final Exception exception;
 
+  /** HTTP code use in response for non HTTP errors. */
+  private static final int FAKE_HTTP_ERROR_CODE = 999;
+
   public ErrorResponseClientHttpResponse(final Exception e) {
     assert e != null;
     this.exception = e;
@@ -28,13 +31,14 @@ public InputStream getBody() {
 
   @Override
   public int getRawStatusCode() {
-    return 500;
+    return FAKE_HTTP_ERROR_CODE;
   }
 
   @Override
   @Nonnull
   public String getStatusText() {
-    return exception.getMessage();
+    return String.format(
+        "%s: %s", this.exception.getClass().getSimpleName(), this.exception.getMessage());
   }
 
   @Override

diff --git a/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java b/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java
@@ -89,15 +89,23 @@ private CachedClientHttpResponse(final ClientHttpResponse originalResponse) thro
       this.statusText = originalResponse.getStatusText();
       this.cachedFile =
           File.createTempFile("cacheduri", null, HttpRequestFetcher.this.temporaryDirectory);
+      LOGGER.debug("Caching URI resource to {}", this.cachedFile);
       try (OutputStream os = Files.newOutputStream(this.cachedFile.toPath())) {
-        IOUtils.copy(originalResponse.getBody(), os);
+        InputStream body = originalResponse.getBody();
+        LOGGER.debug(
+            "Get from input stream {}, for response {}, body available: {}",
+            body.getClass(),
+            originalResponse.getClass(),
+            body.available());
+        IOUtils.copy(body, os);
       }
     }
 
     @Override
     @Nonnull
     public InputStream getBody() throws IOException {
       if (this.body == null) {
+        LOGGER.debug("Loading cached URI resource from {}", this.cachedFile);
         this.body = new FileInputStream(this.cachedFile);
       }
       return this.body;

diff --git a/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java b/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java
@@ -235,12 +235,14 @@ public void close() {
         getBody();
         if (inputStream != null) {
           inputStream.close();
+          inputStream = null;
         }
       } catch (IOException e) {
         LOGGER.error(
             "Error occurred while trying to retrieve Http Response {} in order to close it.",
             this.id,
             e);
+        inputStream = null;
       }
       LOGGER.trace("Closed Http Response object: {}", this.id);
     }