From 17d5b7b8266eaf63743d7d8e5655dd7257cdf50e Mon Sep 17 00:00:00 2001 From: Benjamin Muschko Date: Fri, 28 Dec 2018 10:54:38 -0600 Subject: [PATCH] Handle URL resolution errors properly Do not panic right away when an URL fails to resolve. Capture the error and make it visible in the statistic. The change also treats the fail CLI options properly so that the programs fails after all links have been processed independent of the outcome. --- file/file_test.go | 7 +++++++ http/http.go | 13 ++++++++----- http/http_test.go | 29 +++++++++++++++++++++++------ stat/stat.go | 11 +++++++++++ stat/stat_test.go | 15 ++++++++++++++- verify/verify.go | 10 +++++++--- 6 files changed, 70 insertions(+), 15 deletions(-) diff --git a/file/file_test.go b/file/file_test.go index 59bb323..d44f12c 100644 --- a/file/file_test.go +++ b/file/file_test.go @@ -108,6 +108,13 @@ func TestReadFile(t *testing.T) { deleteFile(path1) } +func TestPanicWhenReadingNonExistentFile(t *testing.T) { + path := filepath.Join("1.adoc") + Panics(t, func() { + ReadFile(path) + }) +} + func createDir(path string) { err := os.MkdirAll(path, 0755) diff --git a/http/http.go b/http/http.go index ed5525b..d61aff7 100644 --- a/http/http.go +++ b/http/http.go @@ -16,24 +16,26 @@ func SetTimeout(timeout int) { // Get emits a HTTP GET request for a given URL. Captures the status code, status and outcome of the call. // Returns with information about the response. func Get(link string) HttpResponse { - result := HttpResponse{Url: link, Success: true} + result := HttpResponse{Url: link} url, err := url.ParseRequestURI(link) if err != nil { - panic(err) + result.Error = err + return result } resp, err := client.Get(url.String()) if err != nil { - panic(err) + result.Error = err + return result } result.StatusCode = resp.StatusCode result.Status = resp.Status - if resp.StatusCode != 200 { - result.Success = false + if resp.StatusCode == 200 { + result.Success = true } resp.Body.Close() @@ -46,4 +48,5 @@ type HttpResponse struct { Success bool StatusCode int Status string + Error error } diff --git a/http/http_test.go b/http/http_test.go index ccc5e0d..d4a0a43 100644 --- a/http/http_test.go +++ b/http/http_test.go @@ -6,27 +6,44 @@ import ( "testing" ) +func TestSetTimeout(t *testing.T) { + SetTimeout(20) +} + func TestGetValidUrl(t *testing.T) { url := "http://www.google.com/" result := Get(url) Equal(t, url, result.Url) True(t, result.Success) + Nil(t, result.Error) Equal(t, 200, result.StatusCode) } +func TestGetUrlForBadRequest(t *testing.T) { + url := "https://www.googleapis.com/urlshortener/v1/url" + result := Get(url) + + Equal(t, url, result.Url) + False(t, result.Success) + Nil(t, result.Error) + Equal(t, 400, result.StatusCode) +} + func TestGetNonExistentUrl(t *testing.T) { url := "http://www.unknown1x.com/" - Panics(t, func() { - Get(url) - }) + result := Get(url) + + Equal(t, url, result.Url) + False(t, result.Success) + NotNil(t, result.Error) } -func TestGetUrlForBadRequest(t *testing.T) { - url := "https://www.googleapis.com/urlshortener/v1/url" +func TestGetInvalidUrl(t *testing.T) { + url := "123://www.invalid.com/" result := Get(url) Equal(t, url, result.Url) False(t, result.Success) - Equal(t, 400, result.StatusCode) + NotNil(t, result.Error) } diff --git a/stat/stat.go b/stat/stat.go index e6bbfa7..4713ece 100644 --- a/stat/stat.go +++ b/stat/stat.go @@ -20,6 +20,16 @@ func SumFailures(aggregateSummary []Summary) int { return sum(failures) } +// SumErrors sums up all errors. +// Returns the overall number of errors. +func SumErrors(aggregateSummary []Summary) int { + errors := collect(aggregateSummary, func(summary Summary) int { + return summary.Errored + }) + + return sum(errors) +} + func collect(list []Summary, f convert) []int { result := make([]int, len(list)) @@ -46,4 +56,5 @@ type convert func(Summary) int type Summary struct { Successful int Failed int + Errored int } diff --git a/stat/stat_test.go b/stat/stat_test.go index 14883f2..8b652d0 100644 --- a/stat/stat_test.go +++ b/stat/stat_test.go @@ -15,7 +15,7 @@ func TestSumSuccessesForEmptySlice(t *testing.T) { func TestSumSuccessesForPopulatedSlice(t *testing.T) { aggregateSummary := summaries() sum := SumSuccesses(aggregateSummary) - Equal(t, 29, sum) + Equal(t, 41, sum) } func TestSumFailuresForEmptySlice(t *testing.T) { @@ -30,10 +30,23 @@ func TestSumFailuresForPopulatedSlice(t *testing.T) { Equal(t, 70, sum) } +func TestSumErrorsForEmptySlice(t *testing.T) { + aggregateSummary := []Summary{} + sum := SumErrors(aggregateSummary) + Equal(t, 0, sum) +} + +func TestSumErrorsForPopulatedSlice(t *testing.T) { + aggregateSummary := summaries() + sum := SumErrors(aggregateSummary) + Equal(t, 2, sum) +} + func summaries() []Summary { summaries := []Summary{} summaries = append(summaries, Summary{Successful: 20, Failed: 3}) summaries = append(summaries, Summary{Successful: 7, Failed: 67}) summaries = append(summaries, Summary{Successful: 2, Failed: 0}) + summaries = append(summaries, Summary{Successful: 12, Failed: 0, Errored: 2}) return summaries } diff --git a/verify/verify.go b/verify/verify.go index 3441bec..7e14fe3 100644 --- a/verify/verify.go +++ b/verify/verify.go @@ -49,12 +49,13 @@ func Process(files []string, fail bool) { if len(aggregateSummary) > 0 { successCount := stat.SumSuccesses(aggregateSummary) failureCount := stat.SumFailures(aggregateSummary) - stats := fmt.Sprintf("SUCCESSFUL: %s, FAILED: %s", strconv.Itoa(successCount), strconv.Itoa(failureCount)) + errorCount := stat.SumErrors(aggregateSummary) + stats := fmt.Sprintf("SUCCESSFUL: %s, FAILED: %s, ERRORED: %s", strconv.Itoa(successCount), strconv.Itoa(failureCount), strconv.Itoa(errorCount)) fmt.Println() fmt.Println(calculateSeparator(stats)) fmt.Println(stats) - if failureCount > 0 && !fail { + if (failureCount > 0 || errorCount > 0) && !fail { os.Exit(1) } } @@ -84,7 +85,10 @@ func parseLinks(content string) stat.Summary { func validateLink(link string, summary *stat.Summary, ch chan<- string) { response := http.Get(link) - if response.Success { + if response.Error != nil { + summary.Errored++ + ch <- fmt.Sprintf("[ERROR] %s (%s)", link, response.Error.Error()) + } else if response.Success { summary.Successful++ ch <- fmt.Sprintf("[OK] %s", link) } else {