From 18655f8b2a22b87caaf2ed3205d61f7d3211cdac Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sun, 5 Feb 2023 11:42:59 +0000 Subject: [PATCH 1/2] Implementation of output and error handling --- .../fetcher/fetcher.go | 88 +++++++++++++ .../fetcher/fetcher_test.go | 116 ++++++++++++++++++ projects/output-and-error-handling/go.mod | 11 ++ projects/output-and-error-handling/go.sum | 17 +++ projects/output-and-error-handling/main.go | 29 +++++ .../testutils/mock_round_tripper.go | 64 ++++++++++ 6 files changed, 325 insertions(+) create mode 100644 projects/output-and-error-handling/fetcher/fetcher.go create mode 100644 projects/output-and-error-handling/fetcher/fetcher_test.go create mode 100644 projects/output-and-error-handling/go.mod create mode 100644 projects/output-and-error-handling/go.sum create mode 100644 projects/output-and-error-handling/main.go create mode 100644 projects/output-and-error-handling/testutils/mock_round_tripper.go diff --git a/projects/output-and-error-handling/fetcher/fetcher.go b/projects/output-and-error-handling/fetcher/fetcher.go new file mode 100644 index 000000000..88a7bcaa1 --- /dev/null +++ b/projects/output-and-error-handling/fetcher/fetcher.go @@ -0,0 +1,88 @@ +package fetcher + +import ( + "errors" + "fmt" + "io" + "net/http" + "os" + "strconv" + "time" +) + +var ErrRetry = errors.New("should retry") + +type WeatherFetcher struct { + client http.Client +} + +// Fetch gets the weather. If it encounters an error, it will either return retryError if the error is retriable, or another error if it is fatal. +func (w *WeatherFetcher) Fetch(url string) (string, error) { + response, err := w.client.Get(url) + if err != nil { + // Add context to the error about what we were trying to do when we encountered it. + // We don't wrap with something like "couldn't get weather", because our caller is expected to add that kind of context. + return "", fmt.Errorf("couldn't make HTTP request: %w", err) + } + defer response.Body.Close() + switch response.StatusCode { + case http.StatusOK: + body, err := io.ReadAll(response.Body) + if err != nil { + return "", fmt.Errorf("error trying to read response: %w", err) + } + return string(body), nil + case http.StatusTooManyRequests: + if err := handle429(response.Header.Get("retry-after")); err != nil { + return "", fmt.Errorf("error handling 'too many requests' response: %w", err) + } + return "", ErrRetry + default: + errorDescription := convertHTTPErrorResponseToDescription(response) + return "", fmt.Errorf("unexpected response from server: %s", errorDescription) + } +} + +func handle429(retryAfterHeader string) error { + delay, err := parseDelay(retryAfterHeader) + if err != nil { + // handle429 is a really small function that doesn't really do much - its job is "parse a header to seconds, then sleep for that many seconds". + // Accordingly, we don't really have much context to add to the error here, so we won't wrap it. + return err + } + // This code considers each request independently - it would also be very reasonable to keep a timer since when we made the first request, and give up if the _total_ time is going to be more than 5 seconds, rather than the per-request time. + if delay > 5*time.Second { + return fmt.Errorf("giving up request: server told us it's going to be too busy for requests for more than the next 5 seconds") + } + if delay > 1*time.Second { + fmt.Fprintf(os.Stderr, "Server reported it's receiving too many requests - waiting %s before retrying\n", delay) + } + time.Sleep(delay) + return nil +} + +func parseDelay(retryAfterHeader string) (time.Duration, error) { + // Try to decode the retry-after header as a whole number of seconds. + if waitFor, err := strconv.Atoi(retryAfterHeader); err == nil { + return time.Duration(waitFor) / time.Nanosecond * time.Second, nil + } + // If it wasn't a whole number of seconds, maybe it was a timestamp - try to decode that. + if waitUntil, err := http.ParseTime(retryAfterHeader); err == nil { + return time.Until(waitUntil), nil + } + // If we couldn't parse either of the expected forms of the header, give up. + // Include the raw value in the error to help with debugging. + // Note that if this were a web service, we'd probably log the bad value on the server-side, but not return as much information to the user. + return -1, fmt.Errorf("couldn't parse retry-after header as an integer number of seconds or a date. Value was: %q", retryAfterHeader) +} + +func convertHTTPErrorResponseToDescription(response *http.Response) string { + var bodyString string + body, err := io.ReadAll(response.Body) + if err == nil { + bodyString = string(body) + } else { + bodyString = "" + } + return fmt.Sprintf("Status code: %s, Body: %s", response.Status, bodyString) +} diff --git a/projects/output-and-error-handling/fetcher/fetcher_test.go b/projects/output-and-error-handling/fetcher/fetcher_test.go new file mode 100644 index 000000000..6b1948f3c --- /dev/null +++ b/projects/output-and-error-handling/fetcher/fetcher_test.go @@ -0,0 +1,116 @@ +package fetcher + +import ( + "fmt" + "net/http" + "testing" + "time" + + "github.com/CodeYourFuture/immersive-go-course/projects/output-and-error-handling/testutils" + "github.com/stretchr/testify/require" +) + +func TestFetch200(t *testing.T) { + transport := testutils.NewMockRoundTripper(t) + transport.StubResponse(http.StatusOK, "some weather", nil) + defer transport.AssertGotRightCalls() + + f := &WeatherFetcher{client: http.Client{ + Transport: transport, + }} + + weather, err := f.Fetch("http://doesnotexist.com/") + + require.NoError(t, err) + require.Equal(t, "some weather", weather) +} + +func TestFetch429(t *testing.T) { + transport := testutils.NewMockRoundTripper(t) + headers := make(http.Header) + headers.Set("retry-after", "1") + transport.StubResponse(http.StatusTooManyRequests, "server overloaded", headers) + defer transport.AssertGotRightCalls() + + f := &WeatherFetcher{client: http.Client{ + Transport: transport, + }} + + start := time.Now() + _, err := f.Fetch("http://doesnotexist.com/") + elapsed := time.Since(start) + + require.Equal(t, ErrRetry, err) + require.GreaterOrEqual(t, elapsed, 1*time.Second) +} + +func Test500(t *testing.T) { + transport := testutils.NewMockRoundTripper(t) + transport.StubResponse(http.StatusInternalServerError, "Something went wrong", nil) + defer transport.AssertGotRightCalls() + + f := &WeatherFetcher{client: http.Client{ + Transport: transport, + }} + + _, err := f.Fetch("http://doesnotexist.com/") + + require.EqualError(t, err, "unexpected response from server: Status code: 500 Internal Server Error, Body: Something went wrong") +} + +func TestDisconnect(t *testing.T) { + transport := testutils.NewMockRoundTripper(t) + transport.StubErrorResponse(fmt.Errorf("connection failed")) + defer transport.AssertGotRightCalls() + + f := &WeatherFetcher{client: http.Client{ + Transport: transport, + }} + + _, err := f.Fetch("http://doesnotexist.com/") + + require.EqualError(t, err, "couldn't make HTTP request: Get \"http://doesnotexist.com/\": connection failed") +} + +func TestParseDelay(t *testing.T) { + // Generally when testing time, we'd inject a controllable clock rather than really using time.Now(). + futureTime := time.Date(2051, time.February, 1, 14, 00, 01, 0, time.UTC) + futureTimeString := "Wed, 01 Feb 2051 14:00:01 GMT" + + for name, tc := range map[string]struct { + header string + delay time.Duration + err string + }{ + "integer seconds": { + header: "10", + delay: 10 * time.Second, + }, + "decimal seconds": { + header: "10.1", + err: "couldn't parse retry-after header as an integer number of seconds or a date. Value was: \"10.1\"", + }, + "far future date:": { + header: futureTimeString, + delay: time.Until(futureTime), + }, + "empty string": { + header: "", + err: `couldn't parse retry-after header as an integer number of seconds or a date. Value was: ""`, + }, + "some text": { + header: "beep boop", + err: `couldn't parse retry-after header as an integer number of seconds or a date. Value was: "beep boop"`, + }, + } { + t.Run(name, func(t *testing.T) { + delay, err := parseDelay(tc.header) + if tc.err != "" { + require.EqualError(t, err, tc.err) + } else { + require.NoError(t, err) + require.InDelta(t, tc.delay/time.Second, delay/time.Second, 1) + } + }) + } +} diff --git a/projects/output-and-error-handling/go.mod b/projects/output-and-error-handling/go.mod new file mode 100644 index 000000000..ad9b3e164 --- /dev/null +++ b/projects/output-and-error-handling/go.mod @@ -0,0 +1,11 @@ +module github.com/CodeYourFuture/immersive-go-course/projects/output-and-error-handling + +go 1.19 + +require github.com/stretchr/testify v1.8.1 + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/projects/output-and-error-handling/go.sum b/projects/output-and-error-handling/go.sum new file mode 100644 index 000000000..2ec90f70f --- /dev/null +++ b/projects/output-and-error-handling/go.sum @@ -0,0 +1,17 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/projects/output-and-error-handling/main.go b/projects/output-and-error-handling/main.go new file mode 100644 index 000000000..e2b35b4e8 --- /dev/null +++ b/projects/output-and-error-handling/main.go @@ -0,0 +1,29 @@ +package main + +import ( + "errors" + "fmt" + "os" + + "github.com/CodeYourFuture/immersive-go-course/projects/output-and-error-handling/fetcher" +) + +func main() { + f := fetcher.WeatherFetcher{} + // Loop because we may need to retry. + for { + if weather, err := f.Fetch("http://localhost:8080/"); err != nil { + // If we're told to retry, do so. + if errors.Is(err, fetcher.ErrRetry) { + continue + } + // Otherwise tell the user there was an error and give up. + fmt.Fprintf(os.Stderr, "Error getting weather: %v\n", err) + os.Exit(1) + } else { + // Print out the weather and be happy. + fmt.Fprintf(os.Stdout, "%s\n", weather) + break + } + } +} diff --git a/projects/output-and-error-handling/testutils/mock_round_tripper.go b/projects/output-and-error-handling/testutils/mock_round_tripper.go new file mode 100644 index 000000000..5faf83f71 --- /dev/null +++ b/projects/output-and-error-handling/testutils/mock_round_tripper.go @@ -0,0 +1,64 @@ +package testutils + +import ( + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +type responseOrError struct { + response *http.Response + err error +} + +type MockRoundTripper struct { + t *testing.T + responses []responseOrError + requestCount int +} + +func NewMockRoundTripper(t *testing.T) *MockRoundTripper { + return &MockRoundTripper{ + t: t, + } +} + +func (m *MockRoundTripper) StubResponse(statusCode int, body string, header http.Header) { + // We need to stub out a fair bit of the HTTP response in for the Go HTTP client to accept our response. + response := &http.Response{ + Body: io.NopCloser(strings.NewReader(body)), + ContentLength: int64(len(body)), + Header: header, + Proto: "HTTP/1.1", + ProtoMajor: 1, + ProtoMinor: 1, + Status: fmt.Sprintf("%d %s", statusCode, http.StatusText(statusCode)), + StatusCode: statusCode, + } + m.responses = append(m.responses, responseOrError{response: response}) +} + +func (m *MockRoundTripper) StubErrorResponse(err error) { + m.responses = append(m.responses, responseOrError{err: err}) +} + +func (m *MockRoundTripper) AssertGotRightCalls() { + m.t.Helper() + + require.Equalf(m.t, len(m.responses), m.requestCount, "Expected %d requests, got %d", len(m.responses), m.requestCount) +} + +func (m *MockRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { + m.t.Helper() + + if len(m.responses) > m.requestCount+2 { + m.t.Fatalf("MockRoundTripper expected %d requests but got more", len(m.responses)) + } + resp := m.responses[m.requestCount] + m.requestCount += 1 + return resp.response, resp.err +} From 4292b6165b03df6d57db1838eacfb582acb27f3a Mon Sep 17 00:00:00 2001 From: chettriyuvraj <32122172+chettriyuvraj@users.noreply.github.com> Date: Thu, 8 Feb 2024 02:33:53 +0530 Subject: [PATCH 2/2] fix: check if request count exceeds the number of responses (#162) --- .../output-and-error-handling/testutils/mock_round_tripper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/output-and-error-handling/testutils/mock_round_tripper.go b/projects/output-and-error-handling/testutils/mock_round_tripper.go index 5faf83f71..42361b2cd 100644 --- a/projects/output-and-error-handling/testutils/mock_round_tripper.go +++ b/projects/output-and-error-handling/testutils/mock_round_tripper.go @@ -55,7 +55,7 @@ func (m *MockRoundTripper) AssertGotRightCalls() { func (m *MockRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { m.t.Helper() - if len(m.responses) > m.requestCount+2 { + if m.requestCount >= len(m.responses) { m.t.Fatalf("MockRoundTripper expected %d requests but got more", len(m.responses)) } resp := m.responses[m.requestCount]