livepeer / livepeer/go-livepeer
cmd: Refactor main() through helpers.
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 586
- Forks
- 226
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 19
Description
**Is your feature request related to a problem? Please describe.**
- We should prevent the main function from terminating before deferred functions can be executed. Currently we are mainly using `glog.Fatal` which is equivalent to calling `os.Exit(1)`, this exits the program not allowing any deferred functions to run.
- In some statements `panic` is used, which executes deferred functions after throwing but it is not idiomatic to panic in such contexts.
```
panic: The amount of pixels per unit must be greater than 0, provided 0 instead
goroutine 1 [running]:
main.main()
/Users/nico/go/src/github.com/livepeer/go-livepeer/cmd/livepeer/livepeer.go:327 +0x56f6
```
- Idiomatically the main function should be kept short and purely used for startup of the program. Startup of services should be initiated through helpers functions.
- We could refactor the cluttered main to simply `glog.Error + return` instead of `glog.Fatal / os.Exit(1)` but this makes testing the returned error codes in `test_args.sh` impossible
Additionally refactoring main makes it easier to maintain the main function and add functionality for the startup of services as needed.
**Describe the solution you'd like**
Extract the startup of various services into helpers. Such as
`parseFlags`
`startDBservice`
`startEthClient`
`setPriceInfo`
`startMediaServer`
`initS3storage`
`startLivepeerNode`
All error statements should be refactored to match the following pattern
```
glog.Errorf("...")
return
```
If the helper returns it's deferred functions will execute. We can then safely os.Exit() in the main after encountering an error
eg.
```
if err := parseFlags(); err != nil {
os.Exit(1)
}
```
This is the same pattern used by [Cobra](https://github.com/spf13/cobra):
eg.
```
// Execute adds all child commands to the root command and sets flags appropriately.
// This is called by main.main(). It only needs to happen once to the rootCmd.
func Execute() {
if err := rootCmd.Execute(); err != nil {
fmt.Println(err)
os.Exit(1)
}
}
```
**Additional context**
See [this discussion](https://github.com/livepeer/go-livepeer/pull/933#discussion_r297212045) in #933
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in cmd/livepeer/livepeer.go, where the issue's stack trace identifies main, and review test_args.sh to understand the expected error codes. Use the listed startup helpers as the scope and check the discussion in PR #933 before changing the error flow. Done means startup responsibilities are extracted, deferred functions can run on errors, and test_args.sh still verifies the exit codes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- cli
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100