Check error before RetryServerErrors

[kisunji]

Small PR to surface errors when calling NewClient before a nil client has a chance to panic.

[annawinkler]

Hi @kisunji - thank you for this pull request! 💖 Curious if you saw some behavior in the tests that this PR fixes? Is there any additional context you could provide? 🤔

[kisunji]

Hi @kisunji - thank you for this pull request! 💖 Curious if you saw some behavior in the tests that this PR fixes? Is there any additional context you could provide? 🤔

Hey! I think you're in the internal slack thread where a dev got a cryptic panic from the tests.

My patch will allow the test to error properly and give important feedback:

--- FAIL: TestOrganizationTagsList (0.00s)
    helper_test.go:57: missing API token
--- FAIL: TestOrganizationTagsDelete (0.00s)
    helper_test.go:57: missing API token
--- FAIL: TestOrganizationTagsAddWorkspace (0.00s)
    helper_test.go:57: missing API token
--- FAIL: TestVariableSetsList (0.00s)
    helper_test.go:57: missing API token
--- FAIL: TestVariableSetsCreate (0.00s)
    helper_test.go:57: missing API token
...

[sebasslash]

With the new addition in #439 , simply add RetryServerErrors: true to the Config object when calling NewClient()

[kisunji]

With the new addition in #439 , simply add RetryServerErrors: true to the Config object when calling NewClient()

Yeah that works as a solution too! This PR is to prevent panics for someone who naively clones this repo and tries to run go test without setting up their environment:

--- FAIL: TestOrganizationTagsList (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x38 pc=0x1005b51c0]

goroutine 19 [running]:
testing.tRunner.func1.2({0x1006a9060, 0x1008bbdd0})
	/usr/local/go/src/testing/testing.go:1389 +0x1c8
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1392 +0x384
panic({0x1006a9060, 0x1008bbdd0})
	/usr/local/go/src/runtime/panic.go:838 +0x204
github.com/hashicorp/go-tfe.(*Client).RetryServerErrors(...)
	/Users/joseph/Documents/go-tfe/tfe.go:343
github.com/hashicorp/go-tfe.testClient(0x140001a6000)
	/Users/joseph/Documents/go-tfe/helper_test.go:56 +0x30
github.com/hashicorp/go-tfe.TestOrganizationTagsList(0x0?)
	/Users/joseph/Documents/go-tfe/organization_tags_integration_test.go:13 +0x30
testing.tRunner(0x140001a6000, 0x10070a8f8)
	/usr/local/go/src/testing/testing.go:1439 +0x110
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:1486 +0x300
exit status 2
FAIL	github.com/hashicorp/go-tfe	0.157s

[kisunji]

If RetryServerErrors is a sensible default I'm fine closing this PR in favor of adding it to NewClient

[annawinkler]

I would advocate that if someone clones this repo and hasn't setup the required environment variables, we show a helpful message instead. How does that sound?

[brandonc]

I don't see any problem with merging this. I think @annawinkler's suggestion is orthogonal and I created a task to deal with it because I think the test config assumes app.terraform.io, so a little analysis needs to be done.

[github-actions]

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.