Add PerformInvite and refactor how errors get handled (#1158)

* Add PerformInvite and refactor how errors get handled

- Rename `JoinError` to `PerformError`
- Remove `error` from the API function signature entirely. This forces
  errors to be bundled into `PerformError` which makes it easier for callers
  to detect and handle errors. On network errors, HTTP clients will make a
  `PerformError`.

* Unbreak everything; thanks Go!

* Send back JSONResponse according to the PerformError

* Update federation invite code too

5 files changed, 95 insertions, 40 deletions

diff --git a/roomserver/api/api.go b/roomserver/api/api.go
index 967f58ba..26ec8ca1 100644
--- a/roomserver/api/api.go
+++ b/roomserver/api/api.go
@@ -18,11 +18,17 @@ type RoomserverInternalAPI interface {
 		response *InputRoomEventsResponse,
 	) error
 
+	PerformInvite(
+		ctx context.Context,
+		req *PerformInviteRequest,
+		res *PerformInviteResponse,
+	)
+
 	PerformJoin(
 		ctx context.Context,
 		req *PerformJoinRequest,
 		res *PerformJoinResponse,
-	) error
+	)
 
 	PerformLeave(
 		ctx context.Context,
diff --git a/roomserver/api/api_trace.go b/roomserver/api/api_trace.go
index a478eeb9..8645b6f2 100644
--- a/roomserver/api/api_trace.go
+++ b/roomserver/api/api_trace.go
@@ -29,14 +29,22 @@ func (t *RoomserverInternalAPITrace) InputRoomEvents(
 	return err
 }
 
+func (t *RoomserverInternalAPITrace) PerformInvite(
+	ctx context.Context,
+	req *PerformInviteRequest,
+	res *PerformInviteResponse,
+) {
+	t.Impl.PerformInvite(ctx, req, res)
+	util.GetLogger(ctx).Infof("PerformInvite req=%+v res=%+v", js(req), js(res))
+}
+
 func (t *RoomserverInternalAPITrace) PerformJoin(
 	ctx context.Context,
 	req *PerformJoinRequest,
 	res *PerformJoinResponse,
-) error {
-	err := t.Impl.PerformJoin(ctx, req, res)
-	util.GetLogger(ctx).WithError(err).Infof("PerformJoin req=%+v res=%+v", js(req), js(res))
-	return err
+) {
+	t.Impl.PerformJoin(ctx, req, res)
+	util.GetLogger(ctx).Infof("PerformJoin req=%+v res=%+v", js(req), js(res))
 }
 
 func (t *RoomserverInternalAPITrace) PerformLeave(
diff --git a/roomserver/api/input.go b/roomserver/api/input.go
index 6c3c8941..05c981df 100644
--- a/roomserver/api/input.go
+++ b/roomserver/api/input.go
@@ -76,21 +76,9 @@ type TransactionID struct {
 	TransactionID string `json:"id"`
 }
 
-// InputInviteEvent is a matrix invite event received over federation without
-// the usual context a matrix room event would have. We usually do not have
-// access to the events needed to check the event auth rules for the invite.
-type InputInviteEvent struct {
-	RoomVersion     gomatrixserverlib.RoomVersion             `json:"room_version"`
-	Event           gomatrixserverlib.HeaderedEvent           `json:"event"`
-	InviteRoomState []gomatrixserverlib.InviteV2StrippedState `json:"invite_room_state"`
-	SendAsServer    string                                    `json:"send_as_server"`
-	TransactionID   *TransactionID                            `json:"transaction_id"`
-}
-
 // InputRoomEventsRequest is a request to InputRoomEvents
 type InputRoomEventsRequest struct {
-	InputRoomEvents   []InputRoomEvent   `json:"input_room_events"`
-	InputInviteEvents []InputInviteEvent `json:"input_invite_events"`
+	InputRoomEvents []InputRoomEvent `json:"input_room_events"`
 }
 
 // InputRoomEventsResponse is a response to InputRoomEvents
diff --git a/roomserver/api/perform.go b/roomserver/api/perform.go
index 0f5394c9..0b8e6df2 100644
--- a/roomserver/api/perform.go
+++ b/roomserver/api/perform.go
@@ -1,19 +1,57 @@
 package api
 
 import (
+	"fmt"
+	"net/http"
+
+	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
 )
 
-type JoinError int
+type PerformErrorCode int
+
+type PerformError struct {
+	Msg  string
+	Code PerformErrorCode
+}
+
+func (p *PerformError) Error() string {
+	return fmt.Sprintf("%d : %s", p.Code, p.Msg)
+}
+
+// JSONResponse maps error codes to suitable HTTP error codes, defaulting to 500.
+func (p *PerformError) JSONResponse() util.JSONResponse {
+	switch p.Code {
+	case PerformErrorBadRequest:
+		return util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.Unknown(p.Msg),
+		}
+	case PerformErrorNoRoom:
+		return util.JSONResponse{
+			Code: http.StatusNotFound,
+			JSON: jsonerror.NotFound(p.Msg),
+		}
+	case PerformErrorNotAllowed:
+		return util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden(p.Msg),
+		}
+	default:
+		return util.ErrorResponse(p)
+	}
+}
 
 const (
-	// JoinErrorNotAllowed means the user is not allowed to join this room (e.g join_rule:invite or banned)
-	JoinErrorNotAllowed JoinError = 1
-	// JoinErrorBadRequest means the request was wrong in some way (invalid user ID, wrong server, etc)
-	JoinErrorBadRequest JoinError = 2
-	// JoinErrorNoRoom means that the room being joined doesn't exist.
-	JoinErrorNoRoom JoinError = 3
+	// PerformErrorNotAllowed means the user is not allowed to invite/join/etc this room (e.g join_rule:invite or banned)
+	PerformErrorNotAllowed PerformErrorCode = 1
+	// PerformErrorBadRequest means the request was wrong in some way (invalid user ID, wrong server, etc)
+	PerformErrorBadRequest PerformErrorCode = 2
+	// PerformErrorNoRoom means that the room being joined doesn't exist.
+	PerformErrorNoRoom PerformErrorCode = 3
+	// PerformErrorNoOperation means that the request resulted in nothing happening e.g invite->invite or leave->leave.
+	PerformErrorNoOperation PerformErrorCode = 4
 )
 
 type PerformJoinRequest struct {
@@ -26,10 +64,8 @@ type PerformJoinRequest struct {
 type PerformJoinResponse struct {
 	// The room ID, populated on success.
 	RoomID string `json:"room_id"`
-	// The reason why the join failed. Can be blank.
-	Error JoinError `json:"error"`
-	// Debugging description of the error. Always present on failure.
-	ErrMsg string `json:"err_msg"`
+	// If non-nil, the join request failed. Contains more information why it failed.
+	Error *PerformError
 }
 
 type PerformLeaveRequest struct {
@@ -40,6 +76,19 @@ type PerformLeaveRequest struct {
 type PerformLeaveResponse struct {
 }
 
+type PerformInviteRequest struct {
+	RoomVersion     gomatrixserverlib.RoomVersion             `json:"room_version"`
+	Event           gomatrixserverlib.HeaderedEvent           `json:"event"`
+	InviteRoomState []gomatrixserverlib.InviteV2StrippedState `json:"invite_room_state"`
+	SendAsServer    string                                    `json:"send_as_server"`
+	TransactionID   *TransactionID                            `json:"transaction_id"`
+}
+
+type PerformInviteResponse struct {
+	// If non-nil, the invite request failed. Contains more information why it failed.
+	Error *PerformError
+}
+
 // PerformBackfillRequest is a request to PerformBackfill.
 type PerformBackfillRequest struct {
 	// The room to backfill
diff --git a/roomserver/api/wrapper.go b/roomserver/api/wrapper.go
index 97940e0c..b73cd190 100644
--- a/roomserver/api/wrapper.go
+++ b/roomserver/api/wrapper.go
@@ -98,16 +98,20 @@ func SendInvite(
 	rsAPI RoomserverInternalAPI, inviteEvent gomatrixserverlib.HeaderedEvent,
 	inviteRoomState []gomatrixserverlib.InviteV2StrippedState,
 	sendAsServer gomatrixserverlib.ServerName, txnID *TransactionID,
-) error {
-	request := InputRoomEventsRequest{
-		InputInviteEvents: []InputInviteEvent{{
-			Event:           inviteEvent,
-			InviteRoomState: inviteRoomState,
-			RoomVersion:     inviteEvent.RoomVersion,
-			SendAsServer:    string(sendAsServer),
-			TransactionID:   txnID,
-		}},
+) *PerformError {
+	request := PerformInviteRequest{
+		Event:           inviteEvent,
+		InviteRoomState: inviteRoomState,
+		RoomVersion:     inviteEvent.RoomVersion,
+		SendAsServer:    string(sendAsServer),
+		TransactionID:   txnID,
 	}
-	var response InputRoomEventsResponse
-	return rsAPI.InputRoomEvents(ctx, &request, &response)
+	var response PerformInviteResponse
+	rsAPI.PerformInvite(ctx, &request, &response)
+	// we need to do this because many places people will use `var err error` as the return
+	// arg and a nil interface != nil pointer to a concrete interface (in this case PerformError)
+	if response.Error != nil && response.Error.Msg != "" {
+		return response.Error
+	}
+	return nil
 }