Update to use util.JSONResponse
MODIFY
src/github.com/matrix-org/dendrite/clientapi/readers/sync.go
src/github.com/matrix-org/dendrite/clientapi/readers/sync.go
@@ -7,11 +7,8 @@ import (
)
// Sync implements /sync
-func Sync(req *http.Request) (interface{}, *util.HTTPError) {
+func Sync(req *http.Request) util.JSONResponse {
logger := util.GetLogger(req.Context())
logger.Info("Doing stuff...")
- return nil, &util.HTTPError{
- Code: 404,
- Message: "Not implemented yet",
- }
+ return util.MessageResponse(404, "Not implemented yet")
}

MODIFY
src/github.com/matrix-org/dendrite/clientapi/routing/routing.go
src/github.com/matrix-org/dendrite/clientapi/routing/routing.go
@@ -17,11 +17,11 @@ const pathPrefixR0 = "/_matrix/client/r0"
func Setup(servMux *http.ServeMux, httpClient *http.Client) {
apiMux := mux.NewRouter()
r0mux := apiMux.PathPrefix(pathPrefixR0).Subrouter()
- r0mux.Handle("/sync", make("sync", wrap(func(req *http.Request) (interface{}, *util.HTTPError) {
+ r0mux.Handle("/sync", make("sync", wrap(func(req *http.Request) util.JSONResponse {
return readers.Sync(req)
})))
r0mux.Handle("/rooms/{roomID}/send/{eventType}",
- make("send_message", wrap(func(req *http.Request) (interface{}, *util.HTTPError) {
+ make("send_message", wrap(func(req *http.Request) util.JSONResponse {
vars := mux.Vars(req)
return writers.SendMessage(req, vars["roomID"], vars["eventType"])
})),
@@ -38,12 +38,12 @@ func make(metricsName string, h util.JSONRequestHandler) http.Handler {
// jsonRequestHandlerWrapper is a wrapper to allow in-line functions to conform to util.JSONRequestHandler
type jsonRequestHandlerWrapper struct {
- function func(req *http.Request) (interface{}, *util.HTTPError)
+ function func(req *http.Request) util.JSONResponse
}
-func (r *jsonRequestHandlerWrapper) OnIncomingRequest(req *http.Request) (interface{}, *util.HTTPError) {
+func (r *jsonRequestHandlerWrapper) OnIncomingRequest(req *http.Request) util.JSONResponse {
return r.function(req)
}
-func wrap(f func(req *http.Request) (interface{}, *util.HTTPError)) *jsonRequestHandlerWrapper {
+func wrap(f func(req *http.Request) util.JSONResponse) *jsonRequestHandlerWrapper {
return &jsonRequestHandlerWrapper{f}
}

MODIFY
src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go
src/github.com/matrix-org/dendrite/clientapi/writers/sendmessage.go
@@ -7,11 +7,8 @@ import (
)
// SendMessage implements /rooms/{roomID}/send/{eventType}
-func SendMessage(req *http.Request, roomID, eventType string) (interface{}, *util.HTTPError) {
+func SendMessage(req *http.Request, roomID, eventType string) util.JSONResponse {
logger := util.GetLogger(req.Context())
logger.WithField("roomID", roomID).WithField("eventType", eventType).Info("Doing stuff...")
- return nil, &util.HTTPError{
- Code: 404,
- Message: "Not implemented yet",
- }
+ return util.MessageResponse(404, "Not implemented yet")
}

MODIFY
vendor/manifest
vendor/manifest
{
"importpath": "github.com/matrix-org/util",
"repository": "https://github.com/matrix-org/util",
- "revision": "4de125c773716ad380f2f80cc6c04789ef4c906a",
+ "revision": "ccef6dc7c24a7c896d96b433a9107b7c47ecf828",
"branch": "master"
},
{

ADD
vendor/src/github.com/matrix-org/util/context.go
+package util
+
+import (
+ "context"
+
+ log "github.com/Sirupsen/logrus"
+)
+
+// contextKeys is a type alias for string to namespace Context keys per-package.
+type contextKeys string
+
+// ctxValueRequestID is the key to extract the request ID for an HTTP request
+const ctxValueRequestID = contextKeys("requestid")
+
+// GetRequestID returns the request ID associated with this context, or the empty string
+// if one is not associated with this context.
+func GetRequestID(ctx context.Context) string {
+ id := ctx.Value(ctxValueRequestID)
+ if id == nil {
+ return ""
+ }
+ return id.(string)
+}
+
+// ctxValueLogger is the key to extract the logrus Logger.
+const ctxValueLogger = contextKeys("logger")
+
+// GetLogger retrieves the logrus logger from the supplied context. Returns nil if there is no logger.
+func GetLogger(ctx context.Context) *log.Entry {
+ l := ctx.Value(ctxValueLogger)
+ if l == nil {
+ return nil
+ }
+ return l.(*log.Entry)
+}

DELETE
vendor/src/github.com/matrix-org/util/error.go
-package util
-
-import "fmt"
-
-// HTTPError An HTTP Error response, which may wrap an underlying native Go Error.
-type HTTPError struct {
- WrappedError error
- // A human-readable message to return to the client in a JSON response. This
- // is ignored if JSON is supplied.
- Message string
- // HTTP status code.
- Code int
- // JSON represents the JSON that should be serialized and sent to the client
- // instead of the given Message.
- JSON interface{}
-}
-
-func (e HTTPError) Error() string {
- var wrappedErrMsg string
- if e.WrappedError != nil {
- wrappedErrMsg = e.WrappedError.Error()
- }
- return fmt.Sprintf("%s: %d: %s", e.Message, e.Code, wrappedErrMsg)
-}

MODIFY
vendor/src/github.com/matrix-org/util/json.go
vendor/src/github.com/matrix-org/util/json.go
@@ -3,7 +3,6 @@ package util
import (
"context"
"encoding/json"
- "fmt"
"math/rand"
"net/http"
"runtime/debug"
@@ -12,46 +11,51 @@ import (
log "github.com/Sirupsen/logrus"
)
-// contextKeys is a type alias for string to namespace Context keys per-package.
-type contextKeys string
+// JSONResponse represents an HTTP response which contains a JSON body.
+type JSONResponse struct {
+ // HTTP status code.
+ Code int
+ // JSON represents the JSON that should be serialized and sent to the client
+ JSON interface{}
+ // Headers represent any headers that should be sent to the client
+ Headers map[string]string
+}
-// ctxValueRequestID is the key to extract the request ID for an HTTP request
-const ctxValueRequestID = contextKeys("requestid")
+// Is2xx returns true if the Code is between 200 and 299.
+func (r JSONResponse) Is2xx() bool {
+ return r.Code/100 == 2
+}
-// GetRequestID returns the request ID associated with this context, or the empty string
-// if one is not associated with this context.
-func GetRequestID(ctx context.Context) string {
- id := ctx.Value(ctxValueRequestID)
- if id == nil {
- return ""
+// RedirectResponse returns a JSONResponse which 302s the client to the given location.
+func RedirectResponse(location string) JSONResponse {
+ headers := make(map[string]string)
+ headers["Location"] = location
+ return JSONResponse{
+ Code: 302,
+ JSON: struct{}{},
+ Headers: headers,
}
- return id.(string)
}
-// ctxValueLogger is the key to extract the logrus Logger.
-const ctxValueLogger = contextKeys("logger")
-
-// GetLogger retrieves the logrus logger from the supplied context. Returns nil if there is no logger.
-func GetLogger(ctx context.Context) *log.Entry {
- l := ctx.Value(ctxValueLogger)
- if l == nil {
- return nil
+// MessageResponse returns a JSONResponse with a 'message' key containing the given text.
+func MessageResponse(code int, msg string) JSONResponse {
+ return JSONResponse{
+ Code: code,
+ JSON: struct {
+ Message string `json:"message"`
+ }{msg},
}
- return l.(*log.Entry)
}
-// JSONRequestHandler represents an interface that must be satisfied in order to respond to incoming
-// HTTP requests with JSON. The interface returned will be marshalled into JSON to be sent to the client,
-// unless the interface is []byte in which case the bytes are sent to the client unchanged.
-// If an error is returned, a JSON error response will also be returned, unless the error code
-// is a 302 REDIRECT in which case a redirect is sent based on the Message field.
-type JSONRequestHandler interface {
- OnIncomingRequest(req *http.Request) (interface{}, *HTTPError)
+// ErrorResponse returns an HTTP 500 JSONResponse with the stringified form of the given error.
+func ErrorResponse(err error) JSONResponse {
+ return MessageResponse(500, err.Error())
}
-// JSONError represents a JSON API error response
-type JSONError struct {
- Message string `json:"message"`
+// JSONRequestHandler represents an interface that must be satisfied in order to respond to incoming
+// HTTP requests with JSON.
+type JSONRequestHandler interface {
+ OnIncomingRequest(req *http.Request) JSONResponse
}
// Protect panicking HTTP requests from taking down the entire process, and log them using
@@ -67,12 +71,7 @@ func Protect(handler http.HandlerFunc) http.HandlerFunc {
}).Errorf(
"Request panicked!\n%s", debug.Stack(),
)
- jsonErrorResponse(
- w, req, &HTTPError{
- Message: "Internal Server Error",
- Code: 500,
- },
- )
+ respond(w, req, MessageResponse(500, "Internal Server Error"))
}
}()
handler(w, req)
@@ -81,11 +80,11 @@ func Protect(handler http.HandlerFunc) http.HandlerFunc {
// MakeJSONAPI creates an HTTP handler which always responds to incoming requests with JSON responses.
// Incoming http.Requests will have a logger (with a request ID/method/path logged) attached to the Context.
-// This can be accessed via GetLogger(Context). The type of the logger is *log.Entry from github.com/Sirupsen/logrus
+// This can be accessed via GetLogger(Context).
func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc {
return Protect(func(w http.ResponseWriter, req *http.Request) {
reqID := RandomString(12)
- // Set a Logger on the context
+ // Set a Logger and request ID on the context
ctx := context.WithValue(req.Context(), ctxValueLogger, log.WithFields(log.Fields{
"req.method": req.Method,
"req.path": req.URL.Path,
@@ -97,75 +96,39 @@ func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc {
logger := req.Context().Value(ctxValueLogger).(*log.Entry)
logger.Print("Incoming request")
- res, httpErr := handler.OnIncomingRequest(req)
+ res := handler.OnIncomingRequest(req)
// Set common headers returned regardless of the outcome of the request
w.Header().Set("Content-Type", "application/json")
SetCORSHeaders(w)
- if httpErr != nil {
- jsonErrorResponse(w, req, httpErr)
- return
- }
-
- // if they've returned bytes as the response, then just return them rather than marshalling as JSON.
- // This gives handlers an escape hatch if they want to return cached bytes.
- var resBytes []byte
- resBytes, ok := res.([]byte)
- if !ok {
- r, err := json.Marshal(res)
- if err != nil {
- jsonErrorResponse(w, req, &HTTPError{
- Message: "Failed to serialise response as JSON",
- Code: 500,
- })
- return
- }
- resBytes = r
- }
- logger.Print(fmt.Sprintf("Responding (%d bytes)", len(resBytes)))
- w.Write(resBytes)
+ respond(w, req, res)
})
}
-func jsonErrorResponse(w http.ResponseWriter, req *http.Request, httpErr *HTTPError) {
+func respond(w http.ResponseWriter, req *http.Request, res JSONResponse) {
logger := req.Context().Value(ctxValueLogger).(*log.Entry)
- if httpErr.Code == 302 {
- logger.WithField("err", httpErr.Error()).Print("Redirecting")
- http.Redirect(w, req, httpErr.Message, 302)
- return
- }
- logger.WithFields(log.Fields{
- log.ErrorKey: httpErr,
- }).Print("Responding with error")
-
- w.WriteHeader(httpErr.Code) // Set response code
- var err error
- var r []byte
- if httpErr.JSON != nil {
- r, err = json.Marshal(httpErr.JSON)
- if err != nil {
- // failed to marshal the supplied interface. Whine and fallback to the HTTP message.
- logger.WithError(err).Error("Failed to marshal HTTPError.JSON")
- }
+ // Set custom headers
+ if res.Headers != nil {
+ for h, val := range res.Headers {
+ w.Header().Set(h, val)
}
-
- // failed to marshal or no custom JSON was supplied, send message JSON.
- if err != nil || httpErr.JSON == nil {
- r, err = json.Marshal(&JSONError{
- Message: httpErr.Message,
- })
}
+ // Marshal JSON response into raw bytes to send as the HTTP body
+ resBytes, err := json.Marshal(res.JSON)
if err != nil {
- // We should never fail to marshal the JSON error response, but in this event just skip
- // marshalling altogether
- logger.Warn("Failed to marshal error response")
- w.Write([]byte(`{}`))
- return
+ logger.WithError(err).Error("Failed to marshal JSONResponse")
+ // this should never fail to be marshalled so drop err to the floor
+ res = MessageResponse(500, "Internal Server Error")
+ resBytes, _ = json.Marshal(res.JSON)
}
- w.Write(r)
+
+ // Set status code and write the body
+ w.WriteHeader(res.Code)
+ logger.WithField("code", res.Code).Infof("Responding (%d bytes)", len(resBytes))
+ w.Write(resBytes)
}
// WithCORSOptions intercepts all OPTIONS requests and responds with CORS headers. The request handler

MODIFY
vendor/src/github.com/matrix-org/util/json_test.go
vendor/src/github.com/matrix-org/util/json_test.go
@@ -2,6 +2,7 @@ package util
import (
"context"
+ "errors"
"net/http"
"net/http/httptest"
"testing"
@@ -10,10 +11,10 @@ import (
)
type MockJSONRequestHandler struct {
- handler func(req *http.Request) (interface{}, *HTTPError)
+ handler func(req *http.Request) JSONResponse
}
-func (h *MockJSONRequestHandler) OnIncomingRequest(req *http.Request) (interface{}, *HTTPError) {
+func (h *MockJSONRequestHandler) OnIncomingRequest(req *http.Request) JSONResponse {
return h.handler(req)
}
@@ -24,36 +25,27 @@ type MockResponse struct {
func TestMakeJSONAPI(t *testing.T) {
log.SetLevel(log.PanicLevel) // suppress logs in test output
tests := []struct {
- Return interface{}
- Err *HTTPError
+ Return JSONResponse
ExpectCode int
ExpectJSON string
}{
- // Error message return values
- {nil, &HTTPError{nil, "Everything is broken", 500, nil}, 500, `{"message":"Everything is broken"}`},
- // Error JSON return values
- {nil, &HTTPError{nil, "Everything is broken", 500, struct {
- Foo string `json:"foo"`
- }{"yep"}}, 500, `{"foo":"yep"}`},
+ // MessageResponse return values
+ {MessageResponse(500, "Everything is broken"), 500, `{"message":"Everything is broken"}`},
+ // interface return values
+ {JSONResponse{500, MockResponse{"yep"}, nil}, 500, `{"foo":"yep"}`},
// Error JSON return values which fail to be marshalled should fallback to text
- {nil, &HTTPError{nil, "Everything is broken", 500, struct {
+ {JSONResponse{500, struct {
Foo interface{} `json:"foo"`
- }{func(cannotBe, marshalled string) {}}}, 500, `{"message":"Everything is broken"}`},
+ }{func(cannotBe, marshalled string) {}}, nil}, 500, `{"message":"Internal Server Error"}`},
// With different status codes
- {nil, &HTTPError{nil, "Not here", 404, nil}, 404, `{"message":"Not here"}`},
- // Success return values
- {&MockResponse{"yep"}, nil, 200, `{"foo":"yep"}`},
+ {JSONResponse{201, MockResponse{"narp"}, nil}, 201, `{"foo":"narp"}`},
// Top-level array success values
- {[]MockResponse{{"yep"}, {"narp"}}, nil, 200, `[{"foo":"yep"},{"foo":"narp"}]`},
- // raw []byte escape hatch
- {[]byte(`actually bytes`), nil, 200, `actually bytes`},
- // impossible marshal
- {func(cannotBe, marshalled string) {}, nil, 500, `{"message":"Failed to serialise response as JSON"}`},
+ {JSONResponse{200, []MockResponse{{"yep"}, {"narp"}}, nil}, 200, `[{"foo":"yep"},{"foo":"narp"}]`},
}
for _, tst := range tests {
- mock := MockJSONRequestHandler{func(req *http.Request) (interface{}, *HTTPError) {
- return tst.Return, tst.Err
+ mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+ return tst.Return
}}
mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
mockWriter := httptest.NewRecorder()
@@ -69,10 +61,38 @@ func TestMakeJSONAPI(t *testing.T) {
}
}
+func TestMakeJSONAPICustomHeaders(t *testing.T) {
+ mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+ headers := make(map[string]string)
+ headers["Custom"] = "Thing"
+ headers["X-Custom"] = "Things"
+ return JSONResponse{
+ Code: 200,
+ JSON: MockResponse{"yep"},
+ Headers: headers,
+ }
+ }}
+ mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+ mockWriter := httptest.NewRecorder()
+ handlerFunc := MakeJSONAPI(&mock)
+ handlerFunc(mockWriter, mockReq)
+ if mockWriter.Code != 200 {
+ t.Errorf("TestMakeJSONAPICustomHeaders wanted HTTP status 200, got %d", mockWriter.Code)
+ }
+ h := mockWriter.Header().Get("Custom")
+ if h != "Thing" {
+ t.Errorf("TestMakeJSONAPICustomHeaders wanted header 'Custom: Thing' , got 'Custom: %s'", h)
+ }
+ h = mockWriter.Header().Get("X-Custom")
+ if h != "Things" {
+ t.Errorf("TestMakeJSONAPICustomHeaders wanted header 'X-Custom: Things' , got 'X-Custom: %s'", h)
+ }
+}
+
func TestMakeJSONAPIRedirect(t *testing.T) {
log.SetLevel(log.PanicLevel) // suppress logs in test output
- mock := MockJSONRequestHandler{func(req *http.Request) (interface{}, *HTTPError) {
- return nil, &HTTPError{nil, "https://matrix.org", 302, nil}
+ mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+ return RedirectResponse("https://matrix.org")
}}
mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
mockWriter := httptest.NewRecorder()
@@ -87,6 +107,50 @@ func TestMakeJSONAPIRedirect(t *testing.T) {
}
}
+func TestMakeJSONAPIError(t *testing.T) {
+ log.SetLevel(log.PanicLevel) // suppress logs in test output
+ mock := MockJSONRequestHandler{func(req *http.Request) JSONResponse {
+ err := errors.New("oops")
+ return ErrorResponse(err)
+ }}
+ mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil)
+ mockWriter := httptest.NewRecorder()
+ handlerFunc := MakeJSONAPI(&mock)
+ handlerFunc(mockWriter, mockReq)
+ if mockWriter.Code != 500 {
+ t.Errorf("TestMakeJSONAPIError wanted HTTP status 500, got %d", mockWriter.Code)
+ }
+ actualBody := mockWriter.Body.String()
+ expect := `{"message":"oops"}`
+ if actualBody != expect {
+ t.Errorf("TestMakeJSONAPIError wanted body '%s', got '%s'", expect, actualBody)
+ }
+}
+
+func TestIs2xx(t *testing.T) {
+ tests := []struct {
+ Code int
+ Expect bool
+ }{
+ {200, true},
+ {201, true},
+ {299, true},
+ {300, false},
+ {199, false},
+ {0, false},
+ {500, false},
+ }
+ for _, test := range tests {
+ j := JSONResponse{
+ Code: test.Code,
+ }
+ actual := j.Is2xx()
+ if actual != test.Expect {
+ t.Errorf("TestIs2xx wanted %t, got %t", test.Expect, actual)
+ }
+ }
+}
+
func TestGetLogger(t *testing.T) {
log.SetLevel(log.PanicLevel) // suppress logs in test output
entry := log.WithField("test", "yep")
@@ -130,6 +194,32 @@ func TestProtect(t *testing.T) {
}
}
+func TestWithCORSOptions(t *testing.T) {
+ log.SetLevel(log.PanicLevel) // suppress logs in test output
+ mockWriter := httptest.NewRecorder()
+ mockReq, _ := http.NewRequest("OPTIONS", "http://example.com/foo", nil)
+ h := WithCORSOptions(func(w http.ResponseWriter, req *http.Request) {
+ w.WriteHeader(200)
+ w.Write([]byte("yep"))
+ })
+ h(mockWriter, mockReq)
+ if mockWriter.Code != 200 {
+ t.Errorf("TestWithCORSOptions wanted HTTP status 200, got %d", mockWriter.Code)
+ }
+
+ origin := mockWriter.Header().Get("Access-Control-Allow-Origin")
+ if origin != "*" {
+ t.Errorf("TestWithCORSOptions wanted Access-Control-Allow-Origin header '*', got '%s'", origin)
+ }
+
+ // OPTIONS request shouldn't hit the handler func
+ expectBody := ""
+ actualBody := mockWriter.Body.String()
+ if actualBody != expectBody {
+ t.Errorf("TestWithCORSOptions wanted body %s, got %s", expectBody, actualBody)
+ }
+}
+
func TestGetRequestID(t *testing.T) {
log.SetLevel(log.PanicLevel) // suppress logs in test output
reqID := "alphabetsoup"

Update to use util.JSONResponse (#18)