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(log): ETH-1685 instantiate logrus with non-nil io.Writer #2

Closed
wants to merge 4 commits into from

Conversation

ATMackay
Copy link

@ATMackay ATMackay commented Apr 3, 2024

Applications built with github.com/AlluvialFinance/go-utils/app.App appear to be panicking at runtime when built with go 1.22

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x18 pc=0x101110308]

goroutine 1 [running]:
github.com/sirupsen/logrus.(*Entry).write(0x140002ede30)
        /Users/alexandermackay/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:294 +0x148
github.com/sirupsen/logrus.(*Entry).log(0x140002eddc0, 0x3, {0x14000656f30, 0x16})
        /Users/alexandermackay/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:252 +0x43c
github.com/sirupsen/logrus.(*Entry).Log(0x140002eddc0, 0x3, {0x1400005bc58?, 0x1400038bd70?, 0x1025f34bc?})
        /Users/alexandermackay/go/pkg/mod/github.com/sirupsen/[email protected]/entry.go:304 +0x60
github.com/sirupsen/logrus.(*Logger).Log(0x14000900900, 0x3, {0x1400005bc58, 0x1, 0x1})
        /Users/alexandermackay/go/pkg/mod/github.com/sirupsen/[email protected]/logger.go:204 +0x60
github.com/sirupsen/logrus.(*Logger).Warn(0x1400064b4a0?, {0x1400005bc58?, 0x0?, 0x0?})
        /Users/alexandermackay/go/pkg/mod/github.com/sirupsen/[email protected]/logger.go:236 +0x30
github.com/alluvialfinance/alluvial-api/cmd.NewRootCommand.NewRunCommand.func2(0x1400001e008?, {0x10367fe20, 0x0, 0x101b84ed7?})
        /Users/alexandermackay/go/src/github.com/AlluvialFinance/alluvial-api/cmd/run.go:16 +0x68
github.com/spf13/cobra.(*Command).execute(0x1400001e008, {0x10367fe20, 0x0, 0x0})
        /Users/alexandermackay/go/pkg/mod/github.com/spf13/[email protected]/command.go:983 +0x840
github.com/spf13/cobra.(*Command).ExecuteC(0x140002f6908)
        /Users/alexandermackay/go/pkg/mod/github.com/spf13/[email protected]/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(0x101a7af18?)
        /Users/alexandermackay/go/pkg/mod/github.com/spf13/[email protected]/command.go:1039 +0x1c
main.main()
        /Users/alexandermackay/go/src/github.com/AlluvialFinance/alluvial-api/main.go:10 +0x20

Upon closer inspection of the github.com/AlluvialFinance/go-utils/log package, it appears that the logger called in New(cfg *config.Config) is being instantiated without a specifying Logger.Out.

This PR

  • Refactors the log package so that New() returns an interface. We can consider alternative implementations in future but this will also help with unit testing.
  • Instantiates logrus Logger with the default values, including a writer to StdErr.
  • Updates go from 1.20 --> 1.22

@ATMackay ATMackay marked this pull request as ready for review April 3, 2024 12:13
@ATMackay ATMackay requested a review from jdiuwe April 3, 2024 13:32
@ATMackay ATMackay marked this pull request as draft April 3, 2024 16:01
@mischat
Copy link
Member

mischat commented Apr 4, 2024

@ATMackay, i guess this needs to be merged, or is this no longer needed ?

@jdiuwe
Copy link

jdiuwe commented Apr 4, 2024

This PR can be closed.

@ATMackay ATMackay closed this Apr 4, 2024
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.

3 participants