Various error handling improvements and user feedback

src/main/java/io/kamax/mxisd/controller/DefaultExceptionHandler.java

@@ -55,20 +55,29 @@ public class DefaultExceptionHandler {
     }
 
     @ExceptionHandler(InternalServerError.class)
-    public String handle(HttpServletRequest req, InternalServerError e, HttpServletResponse response) {
+    public String handle(HttpServletRequest request, HttpServletResponse response, InternalServerError e) {
         if (StringUtils.isNotBlank(e.getInternalReason())) {
             log.error("Reference #{} - {}", e.getReference(), e.getInternalReason());
         } else {
             log.error("Reference #{}", e);
         }
 
-        return handleGeneric(req, e, response);
+        return handleGeneric(request, response, e);
+    }
+
+    @ExceptionHandler(FeatureNotAvailable.class)
+    public String handle(HttpServletRequest request, HttpServletResponse response, FeatureNotAvailable e) {
+        if (StringUtils.isNotBlank(e.getInternalReason())) {
+            log.error("Feature not available: {}", e.getInternalReason());
+        }
+
+        return handleGeneric(request, response, e);
     }
 
     @ExceptionHandler(MatrixException.class)
-    public String handleGeneric(HttpServletRequest req, MatrixException e, HttpServletResponse response) {
+    public String handleGeneric(HttpServletRequest request, HttpServletResponse response, MatrixException e) {
         response.setStatus(e.getStatus());
-        return handle(req, e.getErrorCode(), e.getError());
+        return handle(request, e.getErrorCode(), e.getError());
     }
 
     @ResponseStatus(HttpStatus.BAD_REQUEST)

src/main/java/io/kamax/mxisd/controller/identity/v1/SessionController.java

@@ -35,6 +35,8 @@ import org.springframework.web.bind.annotation.RequestParam;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import java.net.MalformedURLException;
+import java.net.URL;
 
 import static org.springframework.web.bind.annotation.RequestMethod.GET;
 
@@ -67,7 +69,13 @@ class SessionController {
         ValidationResult r = mgr.validate(sid, secret, token);
         log.info("Session {} was validated", sid);
         if (r.getNextUrl().isPresent()) {
-            String url = srvCfg.getPublicUrl() + r.getNextUrl().get();
+            String url = r.getNextUrl().get();
+            try {
+                url = new URL(url).toString();
+            } catch (MalformedURLException e) {
+                log.info("Session next URL {} is not a valid one, will prepend public URL {}", url, srvCfg.getPublicUrl());
+                url = srvCfg.getPublicUrl() + r.getNextUrl().get();
+            }
             log.info("Session {} validation: next URL is present, redirecting to {}", sid, url);
             return "redirect:" + url;
         } else {

src/main/java/io/kamax/mxisd/directory/DirectoryManager.java

@@ -31,6 +31,7 @@ import io.kamax.mxisd.exception.MatrixException;
 import io.kamax.mxisd.util.GsonUtil;
 import io.kamax.mxisd.util.RestClientUtils;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.utils.URIBuilder;
@@ -88,7 +89,12 @@ public class DirectoryManager {
 
             if (status != 200) {
                 MatrixErrorInfo info = gson.fromJson(body, MatrixErrorInfo.class);
-                throw new MatrixException(status, info.getErrcode(), info.getError());
+                if (StringUtils.equals("M_UNRECOGNIZED", info.getErrcode())) { // FIXME no hardcoding, use Enum
+                    log.warn("Homeserver does not support Directory feature, skipping");
+                } else {
+                    log.error("Homeserver returned an error while performing directory search");
+                    throw new MatrixException(status, info.getErrcode(), info.getError());
+                }
             }
 
             UserDirectorySearchResult resultHs = gson.fromJson(body, UserDirectorySearchResult.class);

src/main/java/io/kamax/mxisd/exception/FeatureNotAvailable.java

@@ -0,0 +1,43 @@
+/*
+ * mxisd - Matrix Identity Server Daemon
+ * Copyright (C) 2017 Maxime Dor
+ *
+ * https://max.kamax.io/
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package io.kamax.mxisd.exception;
+
+import org.apache.http.HttpStatus;
+
+public class FeatureNotAvailable extends MatrixException {
+
+    private String internalReason;
+
+    public FeatureNotAvailable(String internalReason) {
+        super(
+                HttpStatus.SC_INTERNAL_SERVER_ERROR,
+                "M_NOT_AVAILABLE",
+                "This action is currently not available. Contact your administrator to enable it."
+        );
+
+        this.internalReason = internalReason;
+    }
+
+    public String getInternalReason() {
+        return internalReason;
+    }
+
+}

src/main/java/io/kamax/mxisd/matrix/IdentityServerUtils.java

@@ -4,6 +4,7 @@ import com.google.gson.JsonElement;
 import com.google.gson.JsonParseException;
 import com.google.gson.JsonParser;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xbill.DNS.*;
@@ -28,6 +29,10 @@ public class IdentityServerUtils {
     private static JsonParser parser = new JsonParser();
 
     public static boolean isUsable(String remote) {
+        if (StringUtils.isBlank(remote)) {
+            return false;
+        }
+
         try {
             // FIXME use Apache HTTP client
             HttpURLConnection rootSrvConn = (HttpURLConnection) new URL(
@@ -54,7 +59,7 @@ public class IdentityServerUtils {
             }
 
             return true;
-        } catch (IOException | JsonParseException e) {
+        } catch (IllegalArgumentException | IOException | JsonParseException e) {
             log.info("{} is not a usable Identity Server: {}", remote, e.getMessage());
             return false;
         }

src/main/java/io/kamax/mxisd/session/SessionMananger.java

@@ -272,9 +272,13 @@ public class SessionMananger {
 
         List<String> servers = mxCfg.getIdentity().getServers(policy.getToRemote().getServer());
         if (servers.isEmpty()) {
-            throw new InternalServerError();
+            throw new FeatureNotAvailable("Remote 3PID sessions are enabled but server list is " +
+                    "misconstrued (invalid ID or empty list");
         }
-        String url = IdentityServerUtils.findIsUrlForDomain(servers.get(0)).orElseThrow(InternalServerError::new);
+
+        String is = servers.get(0);
+        String url = IdentityServerUtils.findIsUrlForDomain(is)
+                .orElseThrow(() -> new InternalServerError(is + " could not be resolved to an Identity server"));
         log.info("Will use IS endpoint {}", url);
 
         String remoteSecret = session.isRemote() ? session.getRemoteSecret() : RandomStringUtils.randomAlphanumeric(16);

src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqliteStorage.java

@@ -146,8 +146,8 @@ public class OrmLiteSqliteStorage implements IStorage {
         return withCatcher(() -> {
             List<ThreePidSessionDao> daoList = sessionDao.queryForMatchingArgs(new ThreePidSessionDao(tpid, secret));
             if (daoList.size() > 1) {
-                log.error("Lookup for 3PID Session {}:{} returned more than one result");
-                throw new InternalServerError();
+                throw new InternalServerError("Lookup for 3PID Session " +
+                        tpid + " returned more than one result");
             }
 
             if (daoList.isEmpty()) {

src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSendGridNotificationHandler.java

@@ -26,11 +26,13 @@ import io.kamax.matrix.ThreePidMedium;
 import io.kamax.mxisd.config.MatrixConfig;
 import io.kamax.mxisd.config.ServerConfig;
 import io.kamax.mxisd.config.threepid.connector.EmailSendGridConfig;
+import io.kamax.mxisd.exception.FeatureNotAvailable;
 import io.kamax.mxisd.invitation.IThreePidInviteReply;
 import io.kamax.mxisd.notification.INotificationHandler;
 import io.kamax.mxisd.threepid.notification.PlaceholderNotificationGenerator;
 import io.kamax.mxisd.threepid.session.IThreePidSession;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -118,6 +120,11 @@ public class EmailSendGridNotificationHandler extends PlaceholderNotificationGen
     }
 
     private void send(String recipient, Email email) {
+        if (StringUtils.isBlank(cfg.getIdentity().getFrom())) {
+            throw new FeatureNotAvailable("3PID Email identity: sender address is empty - " +
+                    "You must set a value for notifications to work");
+        }
+
         try {
             email.addTo(recipient);
             email.setFrom(cfg.getIdentity().getFrom());

src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSmtpConnector.java

@@ -23,6 +23,7 @@ package io.kamax.mxisd.threepid.connector.email;
 import com.sun.mail.smtp.SMTPTransport;
 import io.kamax.matrix.ThreePidMedium;
 import io.kamax.mxisd.config.threepid.connector.EmailSmtpConfig;
+import io.kamax.mxisd.exception.FeatureNotAvailable;
 import io.kamax.mxisd.exception.InternalServerError;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
@@ -66,6 +67,11 @@ public class EmailSmtpConnector implements IEmailConnector {
 
     @Override
     public void send(String senderAddress, String senderName, String recipient, String content) {
+        if (StringUtils.isBlank(senderAddress)) {
+            throw new FeatureNotAvailable("3PID Email identity: sender address is empty - " +
+                    "You must set a value for notifications to work");
+        }
+
         if (StringUtils.isBlank(content)) {
             throw new InternalServerError("Notification content is empty");
         }

src/main/resources/application.yaml

@@ -1,3 +1,8 @@
+# DO NOT USE THIS FILE AS-IS FOR YOUR INITIAL CONFIGURATION
+# ONLY TAKE THE SPECIFIC SECTION YOU WANT TO CONFIGURE
+#
+# For more information about configuration, visit https://github.com/kamax-io/mxisd/blob/master/docs/configure.md
+
 spring:
   main:
     banner-mode: 'off'