livepeer / livepeer/go-livepeer

cmd: Refactor main() through helpers.

Open
#940 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: icebox type: refactor type: tech debt
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.