Skip to content

Commit

Permalink
Merge pull request #217 from promhippie/nil-pointers
Browse files Browse the repository at this point in the history
fix: resolve nil pointer errors
  • Loading branch information
tboerger committed Jun 9, 2023
2 parents 1c08846 + 01de28d commit 8e49869
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 23 deletions.
7 changes: 7 additions & 0 deletions changelog/unreleased/fix-nil-pointer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Bugfix: Resolve nil pointer issues with responses

We have introduced previously a feature where we made sure to close all response
bodies, but we forgot to properly check if these responses are really valid
which lead to nil pointer dereference issues.

https://github.com/promhippie/github_exporter/issues/216
2 changes: 1 addition & 1 deletion pkg/exporter/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func (c *AdminCollector) Collect(ch chan<- prometheus.Metric) {
now := time.Now()
record, resp, err := c.client.Admin.GetAdminStats(ctx)
c.duration.WithLabelValues("admin").Observe(time.Since(now).Seconds())
defer resp.Body.Close()
defer closeBody(resp)

if err != nil {
level.Error(c.logger).Log(
Expand Down
12 changes: 6 additions & 6 deletions pkg/exporter/billing.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func (c *BillingCollector) getActionBilling() []*actionBilling {
continue
}

defer resp.Body.Close()
defer closeBody(resp)

result = append(result, &actionBilling{
Type: "enterprise",
Expand All @@ -390,7 +390,7 @@ func (c *BillingCollector) getActionBilling() []*actionBilling {

for _, name := range c.config.Orgs.Value() {
record, resp, err := c.client.Billing.GetActionsBillingOrg(ctx, name)
defer resp.Body.Close()
defer closeBody(resp)

if err != nil {
level.Error(c.logger).Log(
Expand Down Expand Up @@ -460,7 +460,7 @@ func (c *BillingCollector) getPackageBilling() []*packageBilling {
continue
}

defer resp.Body.Close()
defer closeBody(resp)

result = append(result, &packageBilling{
Type: "enterprise",
Expand All @@ -474,7 +474,7 @@ func (c *BillingCollector) getPackageBilling() []*packageBilling {
defer cancel()

record, resp, err := c.client.Billing.GetPackagesBillingOrg(ctx, name)
defer resp.Body.Close()
defer closeBody(resp)

if err != nil {
level.Error(c.logger).Log(
Expand Down Expand Up @@ -544,7 +544,7 @@ func (c *BillingCollector) getStorageBilling() []*storageBilling {
continue
}

defer resp.Body.Close()
defer closeBody(resp)

result = append(result, &storageBilling{
Type: "enterprise",
Expand All @@ -558,7 +558,7 @@ func (c *BillingCollector) getStorageBilling() []*storageBilling {
defer cancel()

record, resp, err := c.client.Billing.GetStorageBillingOrg(ctx, name)
defer resp.Body.Close()
defer closeBody(resp)

if err != nil {
level.Error(c.logger).Log(
Expand Down
12 changes: 9 additions & 3 deletions pkg/exporter/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import (
"github.com/google/go-github/v53/github"
)

func closeBody(resp *github.Response) {
if resp != nil {
resp.Body.Close()
}
}

func alreadyCollected(collected []string, needle string) bool {
for _, val := range collected {
if needle == val {
Expand Down Expand Up @@ -46,7 +52,7 @@ func reposByOwnerAndName(ctx context.Context, client *github.Client, owner, repo
)

if err != nil {
resp.Body.Close()
closeBody(resp)
return nil, err
}

Expand All @@ -56,11 +62,11 @@ func reposByOwnerAndName(ctx context.Context, client *github.Client, owner, repo
)

if resp.NextPage == 0 {
resp.Body.Close()
closeBody(resp)
break
}

resp.Body.Close()
closeBody(resp)
opts.Page = resp.NextPage
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/exporter/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (c *OrgCollector) Collect(ch chan<- prometheus.Metric) {
now := time.Now()
record, resp, err := c.client.Organizations.Get(ctx, name)
c.duration.WithLabelValues("org").Observe(time.Since(now).Seconds())
defer resp.Body.Close()
defer closeBody(resp)

if err != nil {
level.Error(c.logger).Log(
Expand Down
18 changes: 9 additions & 9 deletions pkg/exporter/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ func (c *RunnerCollector) pagedRepoRunners(ctx context.Context, owner, name stri
)

if err != nil {
resp.Body.Close()
closeBody(resp)
return nil, err
}

Expand All @@ -403,11 +403,11 @@ func (c *RunnerCollector) pagedRepoRunners(ctx context.Context, owner, name stri
)

if resp.NextPage == 0 {
resp.Body.Close()
closeBody(resp)
break
}

resp.Body.Close()
closeBody(resp)
opts.Page = resp.NextPage
}

Expand Down Expand Up @@ -462,7 +462,7 @@ func (c *RunnerCollector) pagedEnterpriseRunners(ctx context.Context, name strin
)

if err != nil {
resp.Body.Close()
closeBody(resp)
return nil, err
}

Expand All @@ -472,11 +472,11 @@ func (c *RunnerCollector) pagedEnterpriseRunners(ctx context.Context, name strin
)

if resp.NextPage == 0 {
resp.Body.Close()
closeBody(resp)
break
}

resp.Body.Close()
closeBody(resp)
opts.Page = resp.NextPage
}

Expand Down Expand Up @@ -531,7 +531,7 @@ func (c *RunnerCollector) pagedOrgRunners(ctx context.Context, name string) ([]*
)

if err != nil {
resp.Body.Close()
closeBody(resp)
return nil, err
}

Expand All @@ -541,11 +541,11 @@ func (c *RunnerCollector) pagedOrgRunners(ctx context.Context, name string) ([]*
)

if resp.NextPage == 0 {
resp.Body.Close()
closeBody(resp)
break
}

resp.Body.Close()
closeBody(resp)
opts.Page = resp.NextPage
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/exporter/workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (c *WorkflowCollector) pagedRepoWorkflows(ctx context.Context, owner, name
)

if err != nil {
resp.Body.Close()
closeBody(resp)
return nil, err
}

Expand All @@ -259,11 +259,11 @@ func (c *WorkflowCollector) pagedRepoWorkflows(ctx context.Context, owner, name
)

if resp.NextPage == 0 {
resp.Body.Close()
closeBody(resp)
break
}

resp.Body.Close()
closeBody(resp)
opts.Page = resp.NextPage
}

Expand Down

0 comments on commit 8e49869

Please sign in to comment.