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.

Sign up for GitHub

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

Jump to bottom

Refactor error handling #303

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Refactor error handling #303

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0f9b045
commands: Rework newQemuHelper to bubble-up error instead of panicing
obbardc Jan 7, 2022
a3fbde3
Rework error handling to check context.State
obbardc Jul 26, 2023
64d043d
Fix typo in stage name Postmachine→PostMachine
obbardc Jan 5, 2022
aa1e5a0
debos: Improve error messages
obbardc Mar 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 29 additions & 28 deletions cmd/debos/debos.go
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mention "this paradigm is quite fragile" but all this change seems to do is rather then having a local variable track the error state (in the form of error code) you moved it to marking it in the context. I'm not really sure why that's less fragile ;)

it does make the code a big more clear though so seems reasonable

Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ import (
"github.com/jessevdk/go-flags"
)

func checkError(context *debos.DebosContext, err error, a debos.Action, stage string) int {
func handleError(context *debos.DebosContext, err error, a debos.Action, stage string) bool {
if err == nil {
return 0
return false
}

context.State = debos.Failed
log.Printf("Action `%s` failed at stage %s, error: %s", a, stage, err)
debos.DebugShell(*context)
obbardc marked this conversation as resolved.
Show resolved Hide resolved
return 1
return true
}

func do_run(r actions.Recipe, context *debos.DebosContext) int {
func do_run(r actions.Recipe, context *debos.DebosContext) bool {
for _, a := range r.Actions {
log.Printf("==== %s ====\n", a)
err := a.Run(context)
Expand All @@ -36,12 +36,12 @@ func do_run(r actions.Recipe, context *debos.DebosContext) int {
defer a.Cleanup(context)

// Check the state of Run method
if exitcode := checkError(context, err, a, "Run"); exitcode != 0 {
return exitcode
if handleError(context, err, a, "Run") {
return false
}
}

return 0
return true
}

func warnLocalhost(variable string, value string) {
Expand Down Expand Up @@ -89,11 +89,12 @@ func main() {
"no_proxy",
}

var exitcode int = 0
// Allow to run all deferred calls prior to os.Exit()
defer func() {
os.Exit(exitcode)
}()
defer func(context debos.DebosContext) {
if context.State == debos.Failed {
os.Exit(1)
}
}(context)

parser := flags.NewParser(&options, flags.Default)
fakemachineBackends := parser.FindOptionByLongName("fakemachine-backend")
Expand All @@ -105,20 +106,20 @@ func main() {
if ok && flagsErr.Type == flags.ErrHelp {
return
} else {
exitcode = 1
context.State = debos.Failed
return
}
}

if len(args) != 1 {
log.Println("No recipe given!")
exitcode = 1
context.State = debos.Failed
return
}

if options.DisableFakeMachine && options.Backend != "auto" {
log.Println("--disable-fakemachine and --fakemachine-backend are mutually exclusive")
exitcode = 1
context.State = debos.Failed
return
}

Expand All @@ -141,12 +142,12 @@ func main() {
r := actions.Recipe{}
if _, err := os.Stat(file); os.IsNotExist(err) {
log.Println(err)
exitcode = 1
context.State = debos.Failed
return
}
if err := r.Parse(file, options.PrintRecipe, options.Verbose, options.TemplateVars); err != nil {
log.Println(err)
exitcode = 1
context.State = debos.Failed
return
}

Expand All @@ -170,7 +171,7 @@ func main() {
if options.Backend == "auto" {
runInFakeMachine = false
} else {
exitcode = 1
context.State = debos.Failed
return
}
}
Expand Down Expand Up @@ -234,7 +235,7 @@ func main() {

for _, a := range r.Actions {
err = a.Verify(&context)
if exitcode = checkError(&context, err, a, "Verify"); exitcode != 0 {
if handleError(&context, err, a, "Verify") {
return
}
}
Expand All @@ -254,7 +255,7 @@ func main() {
memsize, err := units.RAMInBytes(options.Memory)
if err != nil {
fmt.Printf("Couldn't parse memory size: %v\n", err)
obbardc marked this conversation as resolved.
Show resolved Hide resolved
exitcode = 1
context.State = debos.Failed
return
}
m.SetMemory(int(memsize / 1024 / 1024))
Expand All @@ -269,7 +270,7 @@ func main() {
size, err := units.FromHumanSize(options.ScratchSize)
if err != nil {
fmt.Printf("Couldn't parse scratch size: %v\n", err)
obbardc marked this conversation as resolved.
Show resolved Hide resolved
exitcode = 1
context.State = debos.Failed
return
}
m.SetScratch(size, "")
Expand Down Expand Up @@ -311,14 +312,15 @@ func main() {
defer a.PostMachineCleanup(&context)

err = a.PreMachine(&context, m, &args)
if exitcode = checkError(&context, err, a, "PreMachine"); exitcode != 0 {
if handleError(&context, err, a, "PreMachine") {
return
}
}

exitcode, err = m.RunInMachineWithArgs(args)
exitcode, err := m.RunInMachineWithArgs(args)
if err != nil {
fmt.Println(err)
context.State = debos.Failed
return
}

Expand All @@ -329,7 +331,7 @@ func main() {

for _, a := range r.Actions {
err = a.PostMachine(&context)
if exitcode = checkError(&context, err, a, "Postmachine"); exitcode != 0 {
if handleError(&context, err, a, "Postmachine") {
return
}
}
Expand All @@ -344,7 +346,7 @@ func main() {
defer a.PostMachineCleanup(&context)

err = a.PreNoMachine(&context)
if exitcode = checkError(&context, err, a, "PreNoMachine"); exitcode != 0 {
if handleError(&context, err, a, "PreNoMachine") {
return
}
}
Expand All @@ -354,20 +356,19 @@ func main() {
if _, err = os.Stat(context.Rootdir); os.IsNotExist(err) {
err = os.Mkdir(context.Rootdir, 0755)
if err != nil && os.IsNotExist(err) {
exitcode = 1
context.State = debos.Failed
return
}
}

exitcode = do_run(r, &context)
if exitcode != 0 {
if !do_run(r, &context) {
return
}

if !fakemachine.InMachine() {
for _, a := range r.Actions {
err = a.PostMachine(&context)
if exitcode = checkError(&context, err, a, "PostMachine"); exitcode != 0 {
if handleError(&context, err, a, "PostMachine") {
return
}
}
Expand Down
14 changes: 9 additions & 5 deletions commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,11 @@ func (cmd *Command) restoreResolvConf(sum *[sha256.Size]byte) error {
}

func (cmd Command) Run(label string, cmdline ...string) error {
q := newQemuHelper(cmd)
q, err := newQemuHelper(cmd)
if err != nil {
return err
}

q.Setup()
defer q.Cleanup()

Expand Down Expand Up @@ -284,11 +288,11 @@ type qemuHelper struct {
qemutarget string
}

func newQemuHelper(c Command) qemuHelper {
func newQemuHelper(c Command) (*qemuHelper, error) {
q := qemuHelper{}

if c.Chroot == "" || c.Architecture == "" {
return q
return &q, nil
}

switch c.Architecture {
Expand Down Expand Up @@ -323,14 +327,14 @@ func newQemuHelper(c Command) qemuHelper {
q.qemusrc = "/usr/bin/qemu-x86_64-static"
}
default:
log.Panicf("Don't know qemu for Architecture %s", c.Architecture)
return nil, fmt.Errorf("Don't know qemu for architecture %s", c.Architecture)
}

if q.qemusrc != "" {
q.qemutarget = path.Join(c.Chroot, q.qemusrc)
}

return q
return &q, nil
}

func (q qemuHelper) Setup() error {
Expand Down