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

Don't ignore error from http.NewRequest #29

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

flimzy
Copy link

@flimzy flimzy commented Oct 28, 2024

Description

This PR stops ignoring an error, which lead to a nil pointer dereference in my app:

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

goroutine 1 [running]:
github.com/logzio/logzio-go.(*LogzioSender).makeHttpRequest(0xc0000b48f0, {{0xc0001eaa00, 0x19b, 0x200}, 0x0, 0x0}, 0x0, 0x1)
    /go/pkg/mod/github.com/logzio/[email protected]/logziosender.go:300 +0x19b
github.com/logzio/logzio-go.(*LogzioSender).tryToSendLogs(0xc0000b48f0, 0x0)
    /go/pkg/mod/github.com/logzio/[email protected]/logziosender.go:332 +0xe5
github.com/logzio/logzio-go.(*LogzioSender).Drain(0xc0000b48f0)
    /go/pkg/mod/github.com/logzio/[email protected]/logziosender.go:389 +0x28b
github.com/logzio/logzio-go.(*LogzioSender).Stop(0xc0000b48f0)
    /go/pkg/mod/github.com/logzio/[email protected]/logziosender.go:286 +0x56
gitlab.com/packthathouse/backend.git/internal/slog/logzio.(*Handler).Close(...)
    /app/internal/slog/logzio/logzio.go:74
main.run.func1()
    /app/cmd/backend/backend.go:46 +0x3e
main.run()
    /app/cmd/backend/backend.go:53 +0x59a
main.main()
    /app/cmd/backend/backend.go:27 +0x13

No attempt yet to determine why the request was returning an error. One step at a time, and this looks like an obvious oversight regardless of the root cause.

What type of PR is this?

(check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build / CI
  • ⏩ Revert

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help from somebody

@ralongit ralongit self-requested a review October 30, 2024 10:29
@ralongit ralongit changed the base branch from master to release/logzio-go-v1.0.9 October 30, 2024 10:31
@ralongit ralongit merged commit 754cf27 into logzio:release/logzio-go-v1.0.9 Oct 30, 2024
4 checks passed
@ralongit ralongit mentioned this pull request Oct 30, 2024
ralongit added a commit that referenced this pull request Oct 30, 2024
* Bump actions/checkout from 2 to 4

Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

* Bump github.com/shirou/gopsutil/v3 from 3.24.2 to 3.24.5

Bumps [github.com/shirou/gopsutil/v3](https://github.com/shirou/gopsutil) from 3.24.2 to 3.24.5.
- [Release notes](https://github.com/shirou/gopsutil/releases)
- [Commits](shirou/gopsutil@v3.24.2...v3.24.5)

---
updated-dependencies:
- dependency-name: github.com/shirou/gopsutil/v3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Don't ignore error from http.NewRequest (#29)

* Update README.md

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Hall <[email protected]>
@ralongit
Copy link

@flimzy Thank you for your contribution, we have merged the changes(#30) in the logzio-go release v1.0.9.

@flimzy flimzy deleted the ignoredError branch October 30, 2024 14:06
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