leopardslab / gocloud

☁️ Go API for open cloud
Apache License 2.0
119 stars 142 forks source link

Application uses default HTTP Client #48

Open shlokgilda opened 6 years ago

shlokgilda commented 6 years ago

The application uses Go's default HTTP client. However, Go’s http package doesn’t specify request timeouts by default, allowing services to hijack our goroutines. Shouldn't we consider the use of a custom http.Client when connecting to the external APIs?

PratikDhanave commented 6 years ago

hi @shlokgilda can share example of it. It will give more clarification.

shlokgilda commented 6 years ago

Sure!

Issue

Go's default http.Client configures a timeout that short-circuits long-running connections. The default for this value is 0, which is interpreted as “no timeout”. Thus, if the external API service we are calling faces an outrage, the default http.Client would fail. This would cause our connection attempt to hang and they will continue to hang for as long as the malfunctioning server decides to wait. Because API calls were being made to serve user requests, this caused the goroutines serving user requests to hang as well. Once enough users hit the API servicers page, the app fell over, most likely due to resource limits being reached.

For example:

package main

import (
  “fmt”
  “net/http”
  “net/http/httptest”
  “time”
)

func main() {
  svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    time.Sleep(time.Hour)
  }))
  defer svr.Close()
  fmt.Println(“making request”)
  http.Get(svr.URL)
  fmt.Println(“finished request”)
}

When we run this program, it will make a request to a server that will sleep for an hour. Consequently, the program will wait for one hour and then exit.

Solution

We could define a custom http.Client with a defined timeout for our usecase.

var netClient = &http.Client{
  Timeout: time.Second * 10,
}
response, _ := netClient.Get(url)

This sets a 10 second timeout on requests made to the endpoint. If the API server exceeds the timeout, Get() will return with an error.

All in all, a malfunctioning malicious service can hang on to our connection forever, potentially starving our application.

PratikDhanave commented 6 years ago

okay lets work on it. lets figure it out how much time we need to set as gocloud will be used production.

shlokgilda commented 6 years ago

All right. Does this mean refactoring the previous code as well? Also, should any further services being developed use the default client or use a custom http.Client?

PratikDhanave commented 6 years ago

wait we need to make sure this work for all cloud providers .

shlokgilda commented 6 years ago

All right! I am planning to start writing code for DigitalOcean. Should I go ahead with the default http.Client or write a custom client?

PratikDhanave commented 6 years ago

use start with default http.Client

shlokgilda commented 6 years ago

All right. Thanks