From d84705ffd678346e8a9c229a8e3b78d5bd188756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20Gond=C5=BEa?= Date: Thu, 18 Apr 2019 11:52:58 +0200 Subject: [PATCH 1/2] Do not suppress domain exception in favor of follow-up exception --- core/src/main/java/hudson/util/AtomicFileWriter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/hudson/util/AtomicFileWriter.java b/core/src/main/java/hudson/util/AtomicFileWriter.java index c3da305885..ca689c2bbe 100644 --- a/core/src/main/java/hudson/util/AtomicFileWriter.java +++ b/core/src/main/java/hudson/util/AtomicFileWriter.java @@ -205,18 +205,18 @@ public class AtomicFileWriter extends Writer { try { Files.move(tmpPath, destPath, StandardCopyOption.REPLACE_EXISTING); } catch (IOException e1) { - e1.addSuppressed(e); + e.addSuppressed(e1); LOGGER.log(Level.WARNING, "Unable to move {0} to {1}. Attempting to delete {0} and abandoning.", new Path[]{tmpPath, destPath}); try { Files.deleteIfExists(tmpPath); } catch (IOException e2) { - e2.addSuppressed(e1); + e1.addSuppressed(e2); LOGGER.log(Level.WARNING, "Unable to delete {0}, good bye then!", tmpPath); - throw e2; + throw e; } - throw e1; + throw e; } } } From 8b60510cb48d6d190e24cc5f5225843da4b2934b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20Gond=C5=BEa?= Date: Mon, 10 Feb 2020 12:43:21 +0100 Subject: [PATCH 2/2] Clarify what exception gets propagated --- .../java/hudson/util/AtomicFileWriter.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/hudson/util/AtomicFileWriter.java b/core/src/main/java/hudson/util/AtomicFileWriter.java index ca689c2bbe..95415a8b68 100644 --- a/core/src/main/java/hudson/util/AtomicFileWriter.java +++ b/core/src/main/java/hudson/util/AtomicFileWriter.java @@ -189,13 +189,13 @@ public class AtomicFileWriter extends Writer { try { // Try to make an atomic move. Files.move(tmpPath, destPath, StandardCopyOption.ATOMIC_MOVE); - } catch (IOException e) { + } catch (IOException moveFailed) { // If it falls here that can mean many things. Either that the atomic move is not supported, // or something wrong happened. Anyway, let's try to be over-diagnosing - if (e instanceof AtomicMoveNotSupportedException) { - LOGGER.log(Level.WARNING, "Atomic move not supported. falling back to non-atomic move.", e); + if (moveFailed instanceof AtomicMoveNotSupportedException) { + LOGGER.log(Level.WARNING, "Atomic move not supported. falling back to non-atomic move.", moveFailed); } else { - LOGGER.log(Level.WARNING, "Unable to move atomically, falling back to non-atomic move.", e); + LOGGER.log(Level.WARNING, "Unable to move atomically, falling back to non-atomic move.", moveFailed); } if (destPath.toFile().exists()) { @@ -204,19 +204,19 @@ public class AtomicFileWriter extends Writer { try { Files.move(tmpPath, destPath, StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e1) { - e.addSuppressed(e1); + } catch (IOException replaceFailed) { + replaceFailed.addSuppressed(moveFailed); LOGGER.log(Level.WARNING, "Unable to move {0} to {1}. Attempting to delete {0} and abandoning.", new Path[]{tmpPath, destPath}); try { Files.deleteIfExists(tmpPath); - } catch (IOException e2) { - e1.addSuppressed(e2); + } catch (IOException deleteFailed) { + replaceFailed.addSuppressed(deleteFailed); LOGGER.log(Level.WARNING, "Unable to delete {0}, good bye then!", tmpPath); - throw e; + throw replaceFailed; } - throw e; + throw replaceFailed; } } }