From 489514eb60477301eec35365306bfe2b63dd139c Mon Sep 17 00:00:00 2001 From: sebr72 Date: Tue, 17 Dec 2024 19:14:15 +0100 Subject: [PATCH] Close http response, temp file and improve logging --- .../http/ConfigFileResolvingRequest.java | 64 +++++++++++-------- .../print/http/HttpRequestFetcher.java | 22 ++++--- .../job/impl/ThreadPoolJobManager.java | 7 +- 3 files changed, 54 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java b/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java index 59874de66a..332409ce5f 100644 --- a/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java +++ b/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java @@ -141,16 +141,28 @@ private ClientHttpResponse doFileRequest() throws IOException { } private ClientHttpResponse doHttpRequestWithRetry(final HttpHeaders headers) throws IOException { - AtomicInteger counter = new AtomicInteger(); + final AtomicInteger counter = new AtomicInteger(); do { logFetchingURIResource(headers); try { - ClientHttpResponse response = attemptToFetchResponse(headers, counter); + ClientHttpResponse response = attemptToFetchResponse(headers); if (response != null) { return response; + } else { + if (canRetry(counter)) { + sleepWithExceptionHandling(); + LOGGER.debug("Retry fetching {}", this.getURI()); + } else { + PrintException printException = new PrintException("Failed fetching " + getURI()); + LOGGER.debug("Throwing exception since it cannot retry.", printException); + throw printException; + } } } catch (final IOException e) { handleIOException(e, counter); + } catch (final RuntimeException e) { + // handle other exceptions + handleIOException(new IOException(e), counter); } } while (true); } @@ -158,38 +170,33 @@ private ClientHttpResponse doHttpRequestWithRetry(final HttpHeaders headers) thr private void logFetchingURIResource(final HttpHeaders headers) { // Display headers, one by line : LOGGER.debug( - "Fetching URI resource {}, headers:\n{}", + "Fetching {}, using headers:\n{}", this.getURI(), headers.entrySet().stream() .map(entry -> entry.getKey() + "=" + String.join(", ", entry.getValue())) .collect(Collectors.joining("\n"))); } - private ClientHttpResponse attemptToFetchResponse( - final HttpHeaders headers, final AtomicInteger counter) throws IOException { + private ClientHttpResponse attemptToFetchResponse(final HttpHeaders headers) throws IOException { ClientHttpRequest requestUsed = this.request != null ? this.request : createRequestFromWrapped(headers); LOGGER.debug("Executing http request: {}", requestUsed.getURI()); ClientHttpResponse response = executeCallbacksAndRequest(requestUsed); - if (response.getRawStatusCode() < 500) { - LOGGER.debug( - "Fetching success URI resource {}, status code {}", - getURI(), - response.getRawStatusCode()); - return response; - } - LOGGER.debug( - "Fetching failed URI resource {}, error code {}", getURI(), response.getRawStatusCode()); - if (canRetry(counter)) { - sleepWithExceptionHandling(); - LOGGER.debug("Retry fetching URI resource {}", this.getURI()); - } else { - throw new PrintException( - String.format( - "Fetching failed URI resource %s, error code %s", - getURI(), response.getRawStatusCode())); + final int minStatusCodeError = HttpStatus.INTERNAL_SERVER_ERROR.value(); + int rawStatusCode = minStatusCodeError; + try { + rawStatusCode = response.getRawStatusCode(); + if (rawStatusCode < minStatusCodeError) { + LOGGER.debug("Successfully fetched {}, with status code {}", getURI(), rawStatusCode); + return response; + } + LOGGER.debug("Failed fetching {}, with error code {}", getURI(), rawStatusCode); + return null; + } finally { + if (rawStatusCode >= minStatusCodeError) { + response.close(); + } } - return null; } private void handleIOException(final IOException e, final AtomicInteger counter) @@ -197,17 +204,18 @@ private void handleIOException(final IOException e, final AtomicInteger counter) if (canRetry(counter)) { sleepWithExceptionHandling(); - LOGGER.debug("Retry fetching URI resource {}", this.getURI()); + LOGGER.debug("Retry fetching {} following exception", this.getURI(), e); } else { - LOGGER.debug("Fetching failed URI resource {}", getURI()); + LOGGER.debug("Failed fetching {}", getURI()); throw e; } } private void sleepWithExceptionHandling() { + int sleepMs = configFileResolvingHttpRequestFactory.getHttpRequestFetchRetryIntervalMillis(); + LOGGER.debug("Sleeping {} ms before retrying.", sleepMs); try { - TimeUnit.MILLISECONDS.sleep( - configFileResolvingHttpRequestFactory.getHttpRequestFetchRetryIntervalMillis()); + TimeUnit.MILLISECONDS.sleep(sleepMs); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new PrintException("Interrupted while sleeping", e); @@ -215,7 +223,7 @@ private void sleepWithExceptionHandling() { } private boolean canRetry(final AtomicInteger counter) { - return counter.incrementAndGet() < getHttpRequestMaxNumberFetchRetry(); + return counter.incrementAndGet() <= getHttpRequestMaxNumberFetchRetry(); } private int getHttpRequestMaxNumberFetchRetry() { diff --git a/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java b/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java index d8f1827fbc..51a81325f9 100644 --- a/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java +++ b/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java @@ -87,18 +87,22 @@ private CachedClientHttpResponse(final ClientHttpResponse originalResponse) thro this.headers = originalResponse.getHeaders(); this.status = originalResponse.getRawStatusCode(); this.statusText = originalResponse.getStatusText(); - this.cachedFile = + this.cachedFile = createCachedFile(originalResponse.getBody()); + } + + private File createCachedFile(final InputStream originalBody) throws IOException { + File tempFile = File.createTempFile("cacheduri", null, HttpRequestFetcher.this.temporaryDirectory); - LOGGER.debug("Caching URI resource to {}", this.cachedFile); - try (OutputStream os = Files.newOutputStream(this.cachedFile.toPath())) { - InputStream body = originalResponse.getBody(); + LOGGER.debug("Caching URI resource to {}", tempFile); + try (OutputStream os = Files.newOutputStream(tempFile.toPath())) { LOGGER.debug( - "Get from input stream {}, for response {}, body available: {}", - body.getClass(), - originalResponse.getClass(), - body.available()); - IOUtils.copy(body, os); + "Get from input stream {}, body available: {}", + originalBody.getClass(), + originalBody.available()); + IOUtils.copy(originalBody, os); + originalBody.close(); } + return tempFile; } @Override diff --git a/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java b/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java index a6fa026e3b..b5863fdffb 100644 --- a/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java +++ b/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java @@ -452,10 +452,13 @@ private boolean updateRegistry() { final SubmittedPrintJob printJob = submittedJobs.next(); if (!printJob.getReportFuture().isDone() && (isTimeoutExceeded(printJob) || isAbandoned(printJob))) { - LOGGER.info("Canceling job after timeout {}", printJob.getEntry().getReferenceId()); + LOGGER.info( + "About to attempt timeout based automatic cancellation of job {}", + printJob.getEntry().getReferenceId()); if (!printJob.getReportFuture().cancel(true)) { LOGGER.info( - "Could not cancel job after timeout {}", printJob.getEntry().getReferenceId()); + "Automatic cancellation after timeout failed for job {}", + printJob.getEntry().getReferenceId()); } // remove all canceled tasks from the work queue (otherwise the queue comparator // might stumble on non-PrintJob entries)