From 0de7271a3959b64aada834010bdf864685813ca9 Mon Sep 17 00:00:00 2001 From: gkosik Date: Sun, 9 Jun 2019 19:05:57 +0200 Subject: [PATCH] improved logging in the server version of the plugin --- server/client.go | 39 ++++++++++++++++++++++- server/client_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++ server/plugin.go | 31 ++++++++++--------- 3 files changed, 126 insertions(+), 16 deletions(-) diff --git a/server/client.go b/server/client.go index 48554ed69..ec574b923 100644 --- a/server/client.go +++ b/server/client.go @@ -5,8 +5,11 @@ import ( "crypto/tls" "encoding/json" "io" + "io/ioutil" "net/http" "net/url" + + "github.com/pkg/errors" ) type Client struct { @@ -28,8 +31,13 @@ func (c *Client) authenticate(url string, body url.Values) (*AuthResponse, error if err != nil { return nil, err } - var authResponse AuthResponse defer resp.Body.Close() + + if err = c.validateResponse(resp); err != nil { + return nil, err + } + + var authResponse AuthResponse err = json.NewDecoder(resp.Body).Decode(&authResponse) return &authResponse, err } @@ -106,6 +114,35 @@ func (c *Client) do(req *http.Request, v interface{}) (*http.Response, error) { return nil, err } defer resp.Body.Close() + + if err = c.validateResponse(resp); err != nil { + return nil, err + } + err = json.NewDecoder(resp.Body).Decode(v) return resp, err } + +func (c *Client) validateResponse(resp *http.Response) error { + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { + msg := "Bad response received. Status: " + resp.Status + ". " + + xmd := resp.Header.Get("X-Ms-Diagnostics") + if xmd != "" { + msg += "X-Ms-Diagnostics from response: " + xmd + ". " + } else { + msg += "Doesn't have X-Ms-Diagnostics header. " + } + + bodyBytes, _ := ioutil.ReadAll(resp.Body) + if bodyBytes != nil && len(bodyBytes) > 0 { + msg += "Response body: " + string(bodyBytes) + ". " + } else { + msg += "Doesn't have body. " + } + + return errors.New(msg) + } + + return nil +} diff --git a/server/client_test.go b/server/client_test.go index 029f2f938..b67eb5b76 100644 --- a/server/client_test.go +++ b/server/client_test.go @@ -1,17 +1,22 @@ package main import ( + "bytes" "github.com/stretchr/testify/assert" + "io/ioutil" "math" "net/http" "net/http/httptest" "net/url" + "strconv" "testing" ) const ( URL_AUTHENTICATE = "/auth" + URL_AUTHENTICATE_FAILING = "/auth_fail" URL_CREATE_NEW_APP = "/new_app" + URL_CREATE_NEW_APP_FAILING = "/new_app_fail" URL_CREATE_NEW_MEETING = "/new_meeting" URL_PERFORM_DISCOVERY = "/discovery" URL_READ_USER_RESOURCE = "/user" @@ -46,6 +51,11 @@ func TestClient(t *testing.T) { assert.NotNil(t, err) assert.Nil(t, r) + + r, err = client.authenticate(server.URL+URL_AUTHENTICATE_FAILING, url.Values{}) + + assert.NotNil(t, err) + assert.Nil(t, r) }) t.Run("test createNewApplication", func(t *testing.T) { @@ -65,6 +75,11 @@ func TestClient(t *testing.T) { assert.NotNil(t, err) assert.Equal(t, "", r.Embedded.OnlineMeetings.OnlineMeetingsLinks.MyOnlineMeetings.Href) + + r, err = client.createNewApplication(server.URL+URL_CREATE_NEW_APP_FAILING, nil, TEST_TOKEN) + + assert.NotNil(t, err) + assert.Equal(t, "", r.Embedded.OnlineMeetings.OnlineMeetingsLinks.MyOnlineMeetings.Href) }) t.Run("test createNewMeeting", func(t *testing.T) { @@ -109,6 +124,53 @@ func TestClient(t *testing.T) { assert.NotNil(t, err) assert.Nil(t, r) }) + + t.Run("test validateResponse", func(t *testing.T) { + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + + err := client.validateResponse(resp) + + assert.NoError(t, err) + + resp = &http.Response{ + StatusCode: http.StatusCreated, + } + + err = client.validateResponse(resp) + + assert.NoError(t, err) + + resp = &http.Response{ + Status: strconv.Itoa(http.StatusInternalServerError) + " " + http.StatusText(http.StatusInternalServerError), + StatusCode: http.StatusInternalServerError, + Body: ioutil.NopCloser(bytes.NewBufferString("test body")), + } + + err = client.validateResponse(resp) + + assert.Equal( + t, + "Bad response received. Status: 500 Internal Server Error. Doesn't have X-Ms-Diagnostics header. "+ + "Response body: test body. ", + err.Error()) + + resp = &http.Response{ + Status: strconv.Itoa(http.StatusInternalServerError) + " " + http.StatusText(http.StatusInternalServerError), + StatusCode: http.StatusInternalServerError, + Body: ioutil.NopCloser(bytes.NewBufferString("")), + } + + err = client.validateResponse(resp) + + assert.Equal( + t, + "Bad response received. Status: 500 Internal Server Error. Doesn't have X-Ms-Diagnostics header. "+ + "Doesn't have body. ", + err.Error()) + }) } func setupTestServer(t *testing.T) { @@ -118,6 +180,11 @@ func setupTestServer(t *testing.T) { writeResponse(t, writer, `{"access_token": "`+TEST_TOKEN+`"}`) }) + mux.HandleFunc(URL_AUTHENTICATE_FAILING, func(writer http.ResponseWriter, request *http.Request) { + writer.Header().Set("X-Ms-Diagnostics", "wrong credentials") + writer.WriteHeader(http.StatusForbidden) + }) + mux.HandleFunc(URL_CREATE_NEW_APP, func(writer http.ResponseWriter, request *http.Request) { writeResponse(t, writer, ` { @@ -134,6 +201,11 @@ func setupTestServer(t *testing.T) { `) }) + mux.HandleFunc(URL_CREATE_NEW_APP_FAILING, func(writer http.ResponseWriter, request *http.Request) { + writer.Header().Set("X-Ms-Diagnostics", "something went wrong") + writer.WriteHeader(http.StatusInternalServerError) + }) + mux.HandleFunc(URL_CREATE_NEW_MEETING, func(writer http.ResponseWriter, request *http.Request) { writeResponse(t, writer, ` { diff --git a/server/plugin.go b/server/plugin.go index 71e408df9..d6f94ebe2 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -322,13 +322,14 @@ func (p *Plugin) handleRegisterMeetingFromOnlineVersion(w http.ResponseWriter, r func (p *Plugin) handleCreateMeetingInServerVersion(w http.ResponseWriter, r *http.Request) { config := p.getConfiguration() if config.ProductType == PRODUCT_TYPE_ONLINE { + mlog.Error("Cannot create meeting in the server version when the online is set") http.Error(w, "Forbidden", http.StatusForbidden) return } userId := r.Header.Get("Mattermost-User-Id") if userId == "" { - fmt.Println("Request doesn't have Mattermost-User-Id header") + mlog.Error("Request doesn't have Mattermost-User-Id header") http.Error(w, "Not authorized", http.StatusUnauthorized) return } @@ -337,29 +338,29 @@ func (p *Plugin) handleCreateMeetingInServerVersion(w http.ResponseWriter, r *ht var appError *model.AppError user, appError = p.API.GetUser(userId) if appError != nil { - fmt.Println(appError.Error()) + mlog.Error("Error getting user: " + appError.Error()) http.Error(w, appError.Error(), appError.StatusCode) return } else if user == nil { - fmt.Println("User is nil") + mlog.Error("User with that id doesn't exist: " + userId) http.Error(w, "User is nil", http.StatusUnauthorized) return } var req StartServerMeetingRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - fmt.Println(err.Error()) + mlog.Error("Error decoding JSON body: " + err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } else if _, err := p.API.GetChannelMember(req.ChannelId, user.Id); err != nil { - fmt.Println(err.Error()) + mlog.Error("Error getting channel member: " + err.Error()) http.Error(w, "Forbidden", http.StatusForbidden) return } applicationState, apiErr := p.fetchOnlineMeetingsUrl() if apiErr != nil { - fmt.Println(apiErr.Message) + mlog.Error("Error fetching meetings resource url: " + apiErr.Message) http.Error(w, apiErr.Message, http.StatusInternalServerError) return } @@ -372,7 +373,7 @@ func (p *Plugin) handleCreateMeetingInServerVersion(w http.ResponseWriter, r *ht applicationState.Token, ) if err != nil { - fmt.Println(err.Error()) + mlog.Error("Error creating a new meeting: " + err.Error()) http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -397,20 +398,20 @@ func (p *Plugin) handleCreateMeetingInServerVersion(w http.ResponseWriter, r *ht } if post, err := p.API.CreatePost(post); err != nil { - fmt.Println(err.Error()) + mlog.Error("Error creating a new post with the new meeting: " + err.Error()) http.Error(w, err.Error(), http.StatusInternalServerError) return } else { err = p.API.KVSet(fmt.Sprintf("%v%v", POST_MEETING_KEY, newMeetingResponse.MeetingId), []byte(post.Id)) if err != nil { - fmt.Println(err.Error()) + mlog.Error("Error writing meeting id to the database: " + err.Error()) http.Error(w, err.Error(), http.StatusInternalServerError) return } } if err := json.NewEncoder(w).Encode(&newMeetingResponse); err != nil { - fmt.Println(err.Error()) + mlog.Error("Error encoding the new meeting response: " + err.Error()) http.Error(w, err.Error(), http.StatusInternalServerError) } @@ -454,7 +455,7 @@ func (p *Plugin) fetchOnlineMeetingsUrl() (*ApplicationState, *APIError) { applicationState.Token, ) if err != nil { - return nil, &APIError{Message: err.Error()} + return nil, &APIError{Message: "Error creating a new application: " + err.Error()} } applicationState.OnlineMeetingsUrl = "https://" + applicationState.Resource + "/" + newApplicationResponse.Embedded.OnlineMeetings.OnlineMeetingsLinks.MyOnlineMeetings.Href @@ -467,7 +468,7 @@ func (p *Plugin) getApplicationState(discoveryUrl string) (*ApplicationState, *A DiscoveryResponse, err := p.client.performDiscovery(discoveryUrl) if err != nil { - return nil, &APIError{Message: err.Error()} + return nil, &APIError{Message: "Error performing autodiscovery: " + err.Error()} } userResourceUrl := DiscoveryResponse.Links.User.Href @@ -483,12 +484,12 @@ func (p *Plugin) getApplicationState(discoveryUrl string) (*ApplicationState, *A "resource": {resourceName}, }) if err != nil { - return nil, &APIError{Message: err.Error()} + return nil, &APIError{Message: "Error during authentication: " + err.Error()} } userResourceResponse, err := p.client.readUserResource(userResourceUrl, authResponse.Access_token) if err != nil { - return nil, &APIError{Message: err.Error()} + return nil, &APIError{Message: "Error reading user resource: " + err.Error()} } if userResourceResponse.Links.Applications.Href != "" { @@ -501,7 +502,7 @@ func (p *Plugin) getApplicationState(discoveryUrl string) (*ApplicationState, *A return p.getApplicationState(userResourceResponse.Links.Redirect.Href) } else { return nil, &APIError{ - Message: "Unexpected error during creating an application", + Message: "Neither applications resource or redirect resource fetched from user resource", } } }