Skip to content

Commit

Permalink
Various error handling improvements and user feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Maxime Dor committed Oct 7, 2017
1 parent b4f0645 commit 3b697e8
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,29 @@ private String handle(HttpServletRequest req, String erroCode, String error) {
}

@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)
Expand Down
43 changes: 43 additions & 0 deletions src/main/java/io/kamax/mxisd/exception/FeatureNotAvailable.java
Original file line number Diff line number Diff line change
@@ -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;
}

}
7 changes: 6 additions & 1 deletion src/main/java/io/kamax/mxisd/matrix/IdentityServerUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
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.*;
Expand All @@ -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(
Expand All @@ -54,7 +59,7 @@ public static boolean isUsable(String remote) {
}

return true;
} catch (IOException | JsonParseException e) {
} catch (IllegalArgumentException | IOException | JsonParseException e) {
log.info("{} is not a usable Identity Server: {}", remote, e.getMessage());
return false;
}
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/io/kamax/mxisd/session/SessionMananger.java
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,13 @@ public IThreePidSession createRemote(String sid, String secret) {

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ public Optional<IThreePidSessionDao> findThreePidSession(ThreePid tpid, String s
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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@
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;
Expand Down Expand Up @@ -118,6 +120,11 @@ public void sendForRemoteValidation(IThreePidSession session) {
}

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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
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;
Expand Down Expand Up @@ -66,6 +67,11 @@ public String getMedium() {

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

0 comments on commit 3b697e8

Please sign in to comment.