simplify error handling

fixes #6

Signed-off-by: Aleksandar Pesic <peske.nis@gmail.com>

1 files changed, 43 insertions, 85 deletions

diff --git a/internal/http/http.go b/internal/http/http.go
index 1d0ec45..e1f9bdf 100644
--- a/internal/http/http.go
+++ b/internal/http/http.go
@@ -4,16 +4,15 @@ package http
 import (
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
-	"github.com/eduvpn/eduvpn-common/types"
+	"github.com/go-errors/errors"
 )
 
-// The URLParemeters as the name suggests is a type used for the parameters in the URL.
+// URLParameters is a type used for the parameters in the URL.
 type URLParameters map[string]string
 
 // OptionalParams is a structure that defines the optional parameters that are given when making a HTTP call.
@@ -25,26 +24,20 @@ type OptionalParams struct {
 }
 
 // ConstructURL creates a URL with the included parameters.
-func ConstructURL(baseURL string, parameters URLParameters) (string, error) {
-	url, parseErr := url.Parse(baseURL)
-	if parseErr != nil {
-		return "", types.NewWrappedError(
-			fmt.Sprintf(
-				"failed to construct url: %s including parameters: %v",
-				url,
-				parameters,
-			),
-			parseErr,
-		)
+func ConstructURL(baseURL string, params URLParameters) (string, error) {
+	u, err := url.Parse(baseURL)
+	if err != nil {
+		return "", errors.WrapPrefix(err,
+			fmt.Sprintf("failed to construct url '%s' with parameters: %v", u, params), 0)
 	}
 
-	q := url.Query()
+	q := u.Query()
 
-	for parameter, value := range parameters {
-		q.Set(parameter, value)
+	for p, value := range params {
+		q.Set(p, value)
 	}
-	url.RawQuery = q.Encode()
-	return url.String(), nil
+	u.RawQuery = q.Encode()
+	return u.String(), nil
 }
 
 // Get creates a Get request and returns the headers, body and an error.
@@ -52,16 +45,6 @@ func Get(url string) (http.Header, []byte, error) {
 	return MethodWithOpts(http.MethodGet, url, nil)
 }
 
-// Post creates a Post request and returns the headers, body and an error.
-func Post(url string, body url.Values) (http.Header, []byte, error) {
-	return MethodWithOpts(http.MethodGet, url, &OptionalParams{Body: body})
-}
-
-// GetWithOpts creates a Get request with optional parameters and returns the headers, body and an error.
-func GetWithOpts(url string, opts *OptionalParams) (http.Header, []byte, error) {
-	return MethodWithOpts(http.MethodGet, url, opts)
-}
-
 // PostWithOpts creates a Post request with optional parameters and returns the headers, body and an error.
 func PostWithOpts(url string, opts *OptionalParams) (http.Header, []byte, error) {
 	return MethodWithOpts(http.MethodPost, url, opts)
@@ -69,19 +52,12 @@ func PostWithOpts(url string, opts *OptionalParams) (http.Header, []byte, error)
 
 // optionalURL ensures that the URL contains the optional parameters
 // it returns the url (with parameters if success) and an error indicating success.
-func optionalURL(url string, opts *OptionalParams) (string, error) {
-	if opts != nil {
-		url, urlErr := ConstructURL(url, opts.URLParameters)
-
-		if urlErr != nil {
-			return url, types.NewWrappedError(
-				fmt.Sprintf("failed to create HTTP request with url: %s", url),
-				urlErr,
-			)
-		}
-		return url, nil
+func optionalURL(urlStr string, opts *OptionalParams) (string, error) {
+	if opts == nil {
+		return urlStr, nil
 	}
-	return url, nil
+
+	return ConstructURL(urlStr, opts.URLParameters)
 }
 
 // optionalHeaders ensures that the HTTP request uses the optional headers if defined.
@@ -106,16 +82,16 @@ func optionalBodyReader(opts *OptionalParams) io.Reader {
 // It returns the HTTP headers, the body and an error if there is one.
 func MethodWithOpts(
 	method string,
-	url string,
+	urlStr string,
 	opts *OptionalParams,
 ) (http.Header, []byte, error) {
 	// Make sure the url contains all the parameters
 	// This can return an error,
-	// it already has the right error so we don't wrap it further
-	url, urlErr := optionalURL(url, opts)
-	if urlErr != nil {
+	// it already has the right error, so we don't wrap it further
+	urlStr, err := optionalURL(urlStr, opts)
+	if err != nil {
 		// No further type wrapping is needed here
-		return nil, nil, urlErr
+		return nil, nil, err
 	}
 
 	// Default timeout is 5 seconds
@@ -125,15 +101,11 @@ func MethodWithOpts(
 		timeout = opts.Timeout
 	}
 
-	// Create a client
-	client := &http.Client{Timeout: timeout * time.Second}
-
-	errorMessage := fmt.Sprintf("failed HTTP request with method %s and url %s", method, url)
-
 	// Create request object with the body reader generated from the optional arguments
-	req, reqErr := http.NewRequest(method, url, optionalBodyReader(opts))
-	if reqErr != nil {
-		return nil, nil, types.NewWrappedError(errorMessage, reqErr)
+	req, err := http.NewRequest(method, urlStr, optionalBodyReader(opts))
+	if err != nil {
+		return nil, nil, errors.WrapPrefix(err,
+			fmt.Sprintf("failed HTTP request with method %s and url %s", method, urlStr), 0)
 	}
 
 	// See https://stackoverflow.com/questions/17714494/golang-http-request-results-in-eof-errors-when-making-multiple-requests-successi
@@ -142,29 +114,34 @@ func MethodWithOpts(
 	// Make sure the headers contain all the parameters
 	optionalHeaders(req, opts)
 
+	// Create a client
+	c := &http.Client{Timeout: timeout * time.Second}
+
 	// Do request
-	resp, respErr := client.Do(req)
-	if respErr != nil {
-		return nil, nil, types.NewWrappedError(errorMessage, respErr)
+	res, err := c.Do(req)
+	if err != nil {
+		return nil, nil, errors.WrapPrefix(err,
+			fmt.Sprintf("failed HTTP request with method %s and url %s", method, urlStr), 0)
 	}
 
 	// Request successful, make sure body is closed at the end
-	defer resp.Body.Close()
+	defer func() {
+		_ = res.Body.Close()
+	}()
 
 	// Return a string
-	body, readErr := ioutil.ReadAll(resp.Body)
-	if readErr != nil {
-		return resp.Header, nil, types.NewWrappedError(errorMessage, readErr)
+	body, err := io.ReadAll(res.Body)
+	if err != nil {
+		return res.Header, nil, errors.WrapPrefix(err,
+			fmt.Sprintf("failed HTTP request with method %s and url %s", method, urlStr), 0)
 	}
 
-	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		// We make this a custom error because we want to extract the status code later
-		statusErr := &StatusError{URL: url, Body: string(body), Status: resp.StatusCode}
-		return resp.Header, body, types.NewWrappedError(errorMessage, statusErr)
+	if res.StatusCode < 200 || res.StatusCode > 299 {
+		return res.Header, body, errors.Wrap(&StatusError{URL: urlStr, Body: string(body), Status: res.StatusCode}, 0)
 	}
 
 	// Return the body in bytes and signal the status error if there was one
-	return resp.Header, body, nil
+	return res.Header, body, nil
 }
 
 // StatusError indicates that we have received a HTTP status error.
@@ -183,22 +160,3 @@ func (e *StatusError) Error() string {
 		e.Body,
 	)
 }
-
-// ParseJSONError indicates that the HTTP error is because of failed JSON parsing
-// It has the URL and the Body as context.
-// The underlying JSON parsing Err itself is also wrapped here.
-type ParseJSONError struct {
-	URL  string
-	Body string
-	Err  error
-}
-
-// Error returns the ParseJSONError as an error string.
-func (e *ParseJSONError) Error() string {
-	return fmt.Sprintf(
-		"failed parsing json %s for HTTP resource: %s with error: %v",
-		e.Body,
-		e.URL,
-		e.Err,
-	)
-}