Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logic error causing panic storing nil #15

Merged
merged 1 commit into from
Jun 19, 2024
Merged

Fix logic error causing panic storing nil #15

merged 1 commit into from
Jun 19, 2024

Conversation

johnmaguire
Copy link
Member

While writing a test I received the following error:

time="2024-06-18T23:14:56-04:00" level=info msg="update failed" error="failed to call defined networking: failed to make API call to Defined Networking: dnclient endpoint returned bad status code '500', body: unexpected request\n"
panic: sync/atomic: store of nil value into Value

goroutine 113 [running]:
sync/atomic.(*Value).Store(0x0?, {0x0, 0x0})
	/opt/homebrew/Cellar/go/1.22.4/libexec/src/sync/atomic/value.go:49 +0xec
github.com/DefinedNet/dnapi.(*Client).streamingPostDNClient.func1()
	/Users/jmaguire/go/pkg/mod/github.com/!defined!net/dnapi@v0.0.0-20240606181337-15680178c26a/client.go:353 +0x314
created by github.com/DefinedNet/dnapi.(*Client).streamingPostDNClient in goroutine 101
	/Users/jmaguire/go/pkg/mod/github.com/!defined!net/dnapi@v0.0.0-20240606181337-15680178c26a/client.go:326 +0x310
FAIL	github.com/DefinedNet/dnclient	0.212s

Basically, we try to decode a "derp" error when receiving a 500. If we fail to decode it, we store a "generic" error, otherwise we store the fancy typed error.

Except I forgot an "else" so even in error, we'd try to overwrite the generic error with a nil error. It's kind of lucky that atomics don't allow storing nil values, or we might've missed this.

@johnmaguire johnmaguire requested review from IanVS and jasikpark June 19, 2024 14:37
@johnmaguire johnmaguire merged commit b7534ee into main Jun 19, 2024
2 checks passed
@johnmaguire johnmaguire deleted the nil-panic branch June 19, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants