Various error handling improvements and user feedback

This commit is contained in:
Maxime Dor
2017-10-07 06:15:57 +02:00
parent b4f0645257
commit 3b697e86ac
7 changed files with 83 additions and 9 deletions

View File

@@ -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)

View File

@@ -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;
}
}

View File

@@ -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;
}

View File

@@ -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);

View File

@@ -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()) {

View File

@@ -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());

View File

@@ -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");
}