Simple http helper in Go

Question

Can you please share your thoughts on the following simple httphelper in Go? Thanks in advance. I mainly would like to know whether returning a channel of a custom type seems like a good idea and if so , how could I improve it for only receive chans.

package services
import (
 "bytes"
 "encoding/json"
 "io/ioutil"
 "net/http"
 "time"
)
type HHResult struct {
 Result string
 StatusCode int
 Error error
}
type IHttpHelper interface {
 Request(method string, url string, headers map[string][]string, payload interface{}) chan HHResult
 RequestWithOptions(method string, url string, headers map[string][]string, payload interface{}) chan HHResult
}
var singleton IHttpHelper
type HttpHelper struct{}
func (h *HttpHelper) Request(method string, url string, headers map[string][]string, payload interface{}) chan HHResult {
 var result chan HHResult = make(chan HHResult)
 go func(output chan HHResult) {
 payl, err := json.Marshal(payload)
 if err != nil {
 result <- HHResult{
 Result: "",
 StatusCode: 0,
 Error: err,
 }
 }
 req, err := http.NewRequest(method, url, bytes.NewReader(payl))
 client := http.Client{
 Timeout: time.Second * 30,
 }
 resp, err := client.Do(req)
 if err != nil {
 result <- HHResult{
 Result: "",
 StatusCode: 0,
 Error: err,
 }
 }
 respbytes, err := ioutil.ReadAll(resp.Body)
 defer resp.Body.Close()
 result <- HHResult{
 Result: string(respbytes),
 StatusCode: resp.StatusCode,
 Error: nil,
 }
 }(result)
 return result
}
func (h *HttpHelper) RequestWithOptions(method string, url string, headers map[string][]string, payload interface{}, timeout int) chan HHResult {
 var result chan HHResult = make(chan HHResult)
 go func(output chan<- HHResult) {
 payl, err := json.Marshal(payload)
 if err != nil {
 result <- HHResult{
 Result: "",
 StatusCode: 0,
 Error: err,
 }
 }
 req, err := http.NewRequest(method, url, bytes.NewReader(payl))
 client := http.Client{
 Timeout: time.Second * timeout,
 }
 resp, err := client.Do(req)
 if err != nil {
 result <- HHResult{
 Result: "",
 StatusCode: 0,
 Error: err,
 }
 }
 respbytes, err := ioutil.ReadAll(resp.Body)
 defer resp.Body.Close()
 result <- HHResult{
 Result: string(respbytes),
 StatusCode: resp.StatusCode,
 Error: nil,
 }
 }(result)
 return result
}
func NewHttpHelper() IHttpHelper {
 if singleton == nil {
 singleton = &HttpHelper{}
 }
 return singleton
}

Example of usage

func main() {
 helper := services.NewHttpHelper()
 res := <-helper.Request("GET", "https://jsonplaceholder.typicode.com/users/1", nil, nil)
 
 if res.Error != nil {
 fmt.Println(res.Error.Error())
 return
 }
 fmt.Println(res.StatusCode)
 fmt.Println(res.Result)
}

• Coupcoup: Can you explain what your code is trying to do? IHttpHelper and RequestWithOptions aren't particularly descriptive names
• Matias Barrios: they are just a wrapper on top of http operations. I want to GET, POST , etc from and endpoint and get a channel back with the response
• Coupcoup: Can you add a small example of a function that would use this then?
• Matias Barrios: I have added a usage sample.

Answer 1

Listing in order the things I made note of/changed looking through your code:

1. output chan HHResult is declared but never used in go funcs(output chan HHResult). It's fine that it's not used, the go func has access to Result in the parent function, but you don't need to declare it if it's not needed.

2. The headers map argument is never used either.

3. Looking at Request and RequestWithOptions they're the same function except for the variable timeout. Request can be a small wrapper that just sets a default timeout then calls the more general function

4. Similarly, you repeatedly return the same HHResult only substituting which err is used so that can be pulled into it's own function sendErr(chan, err)

5. Any exported function or type (any that start with capital letters) should be commented

6. If you look in the net/http package you'll see it's standard to capitalize HTTP in variable names. Same with JSON.

7. Bugs:

a. One bug in your code is you send JSON via http.Request but you don't actually set the Content-Type to application/json which can cause issues with how your request is interpreted on the other end

b. Another bug is that the error in respbytes, err := ioutil.ReadAll(resp.Body) is never used and you actually return nil for it whether or not you could actually read the body.

c. Timeout: time.Second * 30 throws an error

8. As written, I don't see the need for the IHTTPHelper interface. HttpHelper has the only two methods used hanging directly from the struct and there's no way to inject a different type implementing the same interface into any of your code. As is HttpHelper alone does everything you need.

9. Purely stylistically, I think some of the variable names like HHResults are confusing and rewrote them

---

package services
import (
 "bytes"
 "encoding/json"
 "io/ioutil"
 "net/http"
 "time"
)
// HelperResponse is the response returned by an HTTPHelper.Request()
type HelperResponse struct {
 Result string
 StatusCode int
 Error error
}
// HTTPHelper hangs Request and RequestWithTimeout methods
type HTTPHelper struct{}
var singleton *HTTPHelper
// NewHTTPHelper returns a pointer to the HTTPHelper singleton
func NewHTTPHelper() *HTTPHelper {
 if singleton == nil {
 singleton = &HTTPHelper{}
 }
 return singleton
}
func sendErr(out chan HelperResponse, err error) {
 out <- HelperResponse{
 Result: "",
 StatusCode: 0,
 Error: err,
 }
}
// Request wraps RequestWithTimeout with default 30 second timeout
func (h *HTTPHelper) Request(method string, url string, payload interface{}) (out chan HelperResponse) {
 return h.RequestWithTimeout(method, url, payload, 30)
}
// RequestWithTimeout returns Response, StatusCode, and any Errors from an http request
func (h *HTTPHelper) RequestWithTimeout(method string, url string, payload interface{}, timeout int) (out chan HelperResponse) {
 go func() {
 payloadJSON, err := json.Marshal(payload)
 if err != nil {
 sendErr(out, err)
 }
 req, err := http.NewRequest(method, url, bytes.NewReader(payloadJSON))
 req.Header.Add("Content-Type", "application/json")
 client := http.Client{Timeout: time.Second * time.Duration(timeout)}
 resp, err := client.Do(req)
 if err != nil {
 sendErr(out, err)
 }
 respBytes, err := ioutil.ReadAll(resp.Body)
 defer resp.Body.Close()
 out <- HelperResponse{
 Result: string(respBytes),
 StatusCode: resp.StatusCode,
 Error: err,
 }
 }()
 return out
}

• Matias Barrios: Your comments are awesome. Ill rewrite this!
• Coupcoup: One more note, you'll see in my rewrite I used named returns (out specifically) in the function definition. If you haven't seen that before it saves one line but is pretty much a stylistic choice, not required