Streamline error handling

acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java

@@ -50,6 +50,7 @@ import org.shredzone.acme4j.exception.AcmeRateLimitExceededException;
 import org.shredzone.acme4j.exception.AcmeRetryAfterException;
 import org.shredzone.acme4j.exception.AcmeServerException;
 import org.shredzone.acme4j.exception.AcmeUnauthorizedException;
+import org.shredzone.acme4j.util.AcmeUtils;
 import org.shredzone.acme4j.util.JSON;
 import org.shredzone.acme4j.util.JSONBuilder;
 import org.slf4j.Logger;
@@ -61,10 +62,6 @@ import org.slf4j.LoggerFactory;
 public class DefaultConnection implements Connection {
     private static final Logger LOG = LoggerFactory.getLogger(DefaultConnection.class);
 
-
-    public static final String ACME_ERROR_PREFIX = "urn:ietf:params:acme:error:";
-    private static final String ACME_ERROR_PREFIX_DEPRECATED = "urn:acme:error:";
-
     private static final String ACCEPT_HEADER = "Accept";
     private static final String ACCEPT_CHARSET_HEADER = "Accept-Charset";
     private static final String ACCEPT_LANGUAGE_HEADER = "Accept-Language";
@@ -193,7 +190,12 @@ public class DefaultConnection implements Connection {
             }
 
             JSON json = readJsonResponse();
-            throw createAcmeException(rc, json);
+
+            if (rc == HttpURLConnection.HTTP_CONFLICT) {
+                throw new AcmeConflictException(json.get("detail").asString(), getLocation());
+            }
+
+            throw createAcmeException(json);
         } catch (IOException ex) {
             throw new AcmeNetworkException(ex);
         }
@@ -361,45 +363,35 @@ public class DefaultConnection implements Connection {
 
     /**
      * Handles a problem by throwing an exception. If a JSON problem was returned, an
-     * {@link AcmeServerException} will be thrown. Otherwise a generic
+     * {@link AcmeServerException} or subtype will be thrown. Otherwise a generic
      * {@link AcmeException} is thrown.
      */
-    private AcmeException createAcmeException(int rc, JSON json) {
+    private AcmeException createAcmeException(JSON json) {
         String type = json.get("type").asString();
         String detail = json.get("detail").asString();
-
+        String error = AcmeUtils.stripErrorPrefix(type);
-        if (detail == null) {
-            detail = "general problem";
-        }
-
-        if (rc == HttpURLConnection.HTTP_CONFLICT) {
-            return new AcmeConflictException(detail, getLocation());
-        }
 
         if (type == null) {
             return new AcmeException(detail);
         }
 
-        switch (type) {
+        if ("unauthorized".equals(error)) {
-            case ACME_ERROR_PREFIX + "unauthorized":
+            return new AcmeUnauthorizedException(type, detail);
-            case ACME_ERROR_PREFIX_DEPRECATED + "unauthorized":
-                return new AcmeUnauthorizedException(type, detail);
-
-            case ACME_ERROR_PREFIX + "agreementRequired":
-            case ACME_ERROR_PREFIX_DEPRECATED + "agreementRequired":
-                String instance = json.get("instance").asString();
-                return new AcmeAgreementRequiredException(
-                            type, detail, getLink("terms-of-service"),
-                            instance != null ? resolveRelative(instance) : null);
-
-            case ACME_ERROR_PREFIX + "rateLimited":
-            case ACME_ERROR_PREFIX_DEPRECATED + "rateLimited":
-                return new AcmeRateLimitExceededException(
-                            type, detail, getRetryAfterHeader(), getLinks("rate-limit"));
-
-            default:
-                return new AcmeServerException(type, detail);
         }
+
+        if ("agreementRequired".equals(error)) {
+            URI instance = resolveRelative(json.get("instance").asString());
+            URI tos = getLink("terms-of-service");
+            return new AcmeAgreementRequiredException(type, detail, tos, instance);
+        }
+
+        if ("rateLimited".equals(error)) {
+            Date retryAfter = getRetryAfterHeader();
+            Collection<URI> rateLimits = getLinks("rate-limit");
+            return new AcmeRateLimitExceededException(type, detail, retryAfter, rateLimits);
+        }
+
+        return new AcmeServerException(type, detail);
     }
 
     /**
@@ -439,10 +431,16 @@ public class DefaultConnection implements Connection {
      * Resolves a relative link against the connection's last URI.
      *
      * @param link
-     *            Link to resolve. Absolute links are just converted to an URI.
+     *            Link to resolve. Absolute links are just converted to an URI. May be
-     * @return Absolute URI of the given link.
+     *            {@code null}.
+     * @return Absolute URI of the given link, or {@code null} if the link was
+     *         {@code null}.
      */
     private URI resolveRelative(String link) {
+        if (link == null) {
+            return null;
+        }
+
         assertConnectionIsOpen();
         try {
             return new URL(conn.getURL(), link).toURI();

acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeServerException.java

@@ -15,7 +15,7 @@ package org.shredzone.acme4j.exception;
 
 import java.util.Objects;
 
-import org.shredzone.acme4j.connector.DefaultConnection;
+import org.shredzone.acme4j.util.AcmeUtils;
 
 /**
  * An exception that is thrown when the ACME server returned an error. It contains
@@ -24,8 +24,6 @@ import org.shredzone.acme4j.connector.DefaultConnection;
 public class AcmeServerException extends AcmeException {
     private static final long serialVersionUID = 5971622508467042792L;
 
-    private static final String ACME_ERROR_PREFIX_DEPRECATED = "urn:acme:error:";
-
     private final String type;
 
     /**
@@ -57,13 +55,7 @@ public class AcmeServerException extends AcmeException {
      *         {@code "urn:ietf:params:acme:error"}
      */
     public String getAcmeErrorType() {
-        if (type.startsWith(DefaultConnection.ACME_ERROR_PREFIX)) {
+        return AcmeUtils.stripErrorPrefix(type);
-            return type.substring(DefaultConnection.ACME_ERROR_PREFIX.length());
-        } else if (type.startsWith(ACME_ERROR_PREFIX_DEPRECATED)) {
-            return type.substring(ACME_ERROR_PREFIX_DEPRECATED.length());
-        } else {
-            return null;
-        }
     }
 
 }

acme4j-client/src/main/java/org/shredzone/acme4j/util/AcmeUtils.java

@@ -40,6 +40,8 @@ import org.shredzone.acme4j.exception.AcmeProtocolException;
  */
 public final class AcmeUtils {
     private static final char[] HEX = "0123456789abcdef".toCharArray();
+    private static final String ACME_ERROR_PREFIX = "urn:ietf:params:acme:error:";
+    private static final String ACME_ERROR_PREFIX_DEPRECATED = "urn:acme:error:";
 
     private static final Pattern DATE_PATTERN = Pattern.compile(
                     "^(\\d{4})-(\\d{2})-(\\d{2})T"
@@ -203,4 +205,26 @@ public final class AcmeUtils {
         return cal.getTime();
     }
 
+    /**
+     * Strips the acme error prefix from the error string.
+     * <p>
+     * For example, for "urn:ietf:params:acme:error:conflict", "conflict" is returned.
+     * <p>
+     * This method also handles the deprecated prefix "urn:acme:error:" that is still in
+     * use at Let's Encrypt.
+     *
+     * @param type
+     *            Error type to strip the prefix from. {@code null} is safe.
+     * @return Stripped error type, or {@code null} if the prefix was not found.
+     */
+    public static String stripErrorPrefix(String type) {
+        if (type != null && type.startsWith(ACME_ERROR_PREFIX)) {
+            return type.substring(ACME_ERROR_PREFIX.length());
+        } else if (type != null && type.startsWith(ACME_ERROR_PREFIX_DEPRECATED)) {
+            return type.substring(ACME_ERROR_PREFIX_DEPRECATED.length());
+        } else {
+            return null;
+        }
+    }
+
 }

acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java

@@ -446,7 +446,7 @@ public class DefaultConnectionTest {
         } catch (AcmeNetworkException ex) {
             fail("Did not expect an AcmeNetworkException");
         } catch (AcmeException ex) {
-            assertThat(ex.getMessage(), not(isEmptyOrNullString()));
+            assertThat(ex.getMessage(), isEmptyOrNullString());
         }
 
         verify(mockUrlConnection).getHeaderField("Content-Type");

acme4j-client/src/test/java/org/shredzone/acme4j/util/AcmeUtilsTest.java

@@ -233,6 +233,17 @@ public class AcmeUtilsTest {
         }
     }
 
+    /**
+     * Test that error prefix is correctly removed.
+     */
+    @Test
+    public void testStripErrorPrefix() {
+        assertThat(stripErrorPrefix("urn:ietf:params:acme:error:unauthorized"), is("unauthorized"));
+        assertThat(stripErrorPrefix("urn:acme:error:deprecated"), is("deprecated"));
+        assertThat(stripErrorPrefix("urn:somethingelse:error:message"), is(nullValue()));
+        assertThat(stripErrorPrefix(null), is(nullValue()));
+    }
+
     /**
      * Matches the given time.
      */