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

SD-838: fix panic #33

Merged
merged 2 commits into from
Oct 24, 2024
Merged

SD-838: fix panic #33

merged 2 commits into from
Oct 24, 2024

Conversation

hnnsgstfssn
Copy link

@hnnsgstfssn hnnsgstfssn commented Oct 23, 2024

Description

See https://commercetools.atlassian.net/browse/SD-838 for details.

Fixes a potential panic in the code that has recently been observed.

The change [1] introduced a potential panic as it is trying to log the
app name when a failure happens, but the app variable is overridden
during such a failure and is thus nil.

This results in the following panic.

    panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x20ddccb]

    goroutine 367 [running]:
    internal/pkg/argocd.generateDiffOfAComponent()
	    internal/pkg/argocd/argocd.go:477
    internal/pkg/argocd.GenerateDiffOfChangedComponents()
	    internal/pkg/argocd/argocd.go:561
    internal/pkg/githubapi.HandlePREvent()
	    internal/pkg/githubapi/github.go:161
    internal/pkg/githubapi.handleEvent()
	    internal/pkg/githubapi/github.go:382
    created by internal/pkg/githubapi.ReciveWebhook
	    internal/pkg/githubapi/github.go:322

By instead using the name from the query constructed prior, the panic
should be avoided.

[1] https://github.com/commercetools/telefonistka/commit/adbd91328b2edce48ee4607f101ecb2b6528d77b

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

The change [1] introduced a potential panic as it is trying to log the
app name when a failure happens, but the app variable is overridden
during such a failure and is thus nil.

This results in the following panic.

    panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x20ddccb]

    goroutine 367 [running]:
    internal/pkg/argocd.generateDiffOfAComponent()
	    internal/pkg/argocd/argocd.go:477
    internal/pkg/argocd.GenerateDiffOfChangedComponents()
	    internal/pkg/argocd/argocd.go:561
    internal/pkg/githubapi.HandlePREvent()
	    internal/pkg/githubapi/github.go:161
    internal/pkg/githubapi.handleEvent()
	    internal/pkg/githubapi/github.go:382
    created by internal/pkg/githubapi.ReciveWebhook
	    internal/pkg/githubapi/github.go:322

By instead using the name from the query constructed prior, the panic
should be avoided.

[1] adbd913
Linter complains about using *string for %s argument.
@hnnsgstfssn hnnsgstfssn marked this pull request as ready for review October 24, 2024 13:38
@hnnsgstfssn hnnsgstfssn merged commit 9b32fb1 into main Oct 24, 2024
5 checks passed
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