improved logging in the server version of the plugin

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
+}

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, `
 			{

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",
 		}
 	}
 }