From 7759b7297caca23b975cd6a1c6584b33f7aa06bd Mon Sep 17 00:00:00 2001
From: jcreixell <jorge.creixell@grafana.com>
Date: Wed, 1 Mar 2023 13:59:18 +0100
Subject: [PATCH] Fix loop that never returns on http error

  - While running this exporter in the grafana agent, we have identified a loop
  that never returns in case of http error. This in turn causes the agent to never
  release a lock, preventing it from making progress.
  - Note: I have not added additional metrics to report errors as I am trying to keep the
  scope of this change minimal, but it would be a good idea to add a gauge as stated in
  the [prometheus guidelines](https://prometheus.io/docs/instrumenting/writing_exporters/#failed-scrapes).
---
 exporter/http.go             |  2 +-
 test/github_exporter_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/exporter/http.go b/exporter/http.go
index 12ec206b..037a47fc 100644
--- a/exporter/http.go
+++ b/exporter/http.go
@@ -40,7 +40,7 @@ func asyncHTTPGets(targets []string, token string) ([]*Response, error) {
 		case r := <-ch:
 			if r.err != nil {
 				log.Errorf("Error scraping API, Error: %v", r.err)
-				break
+				return nil, r.err
 			}
 			responses = append(responses, r)
 
diff --git a/test/github_exporter_test.go b/test/github_exporter_test.go
index e3d0fd1e..ae0d6e21 100644
--- a/test/github_exporter_test.go
+++ b/test/github_exporter_test.go
@@ -54,6 +54,22 @@ func TestGithubExporter(t *testing.T) {
 		End()
 }
 
+func TestGithubExporterHttpErrorHandling(t *testing.T) {
+	test, collector := apiTest(withConfig("myOrg/myRepo"))
+	defer prometheus.Unregister(&collector)
+
+	// Test that the exporter returns when an error occurs
+	// Ideally a new gauge should be added to keep track of scrape errors
+	// following prometheus exporter guidelines
+	test.Mocks(
+		githubPullsError(),
+	).
+		Get("/metrics").
+		Expect(t).
+		Status(http.StatusOK).
+		End()
+}
+
 func apiTest(conf config.Config) (*apitest.APITest, exporter.Exporter) {
 	exp := exporter.Exporter{
 		APIMetrics: exporter.AddMetrics(),
@@ -118,6 +134,15 @@ func githubPulls() *apitest.Mock {
 		End()
 }
 
+func githubPullsError() *apitest.Mock {
+	return apitest.NewMock().
+		Get("https://api.github.com/repos/myOrg/myRepo/pulls").
+		Header("Authorization", "token 12345").
+		RespondWith().
+		Status(http.StatusBadRequest).
+		End()
+}
+
 func readFile(path string) string {
 	bytes, err := ioutil.ReadFile(path)
 	if err != nil {