PersistentPostRun{,E} are not executed when RunE or PostRunE returns an error

[Luap99]

the code here contains the main logic when executing a command:

cobra/command.go

Lines 939 to 963 in 4fa4fdf

	if c.RunE != nil {
	if err := c.RunE(c, argWoFlags); err != nil {
	return err
	}
	} else {
	c.Run(c, argWoFlags)
	}
	if c.PostRunE != nil {
	if err := c.PostRunE(c, argWoFlags); err != nil {
	return err
	}
	} else if c.PostRun != nil {
	c.PostRun(c, argWoFlags)
	}
	for p := c; p != nil; p = p.Parent() {
	if p.PersistentPostRunE != nil {
	if err := p.PersistentPostRunE(c, argWoFlags); err != nil {
	return err
	}
	break
	} else if p.PersistentPostRun != nil {
	p.PersistentPostRun(c, argWoFlags)
	break
	}
	}

Both PostRun and PersistentPostRun functions are not executed when the RunE function returns an error.
This is surprising behaviour for us in podman. Given the name we assumed that this function always runs after the run function.

The documentation for the PostRun function is very brief and this behaviour is not mentioned. So as first step we should fix this to help others not making the same mistake.

I understand that cobra is very stable and changing this behaviour could break others. It is also not clear how cobra could handle the case where both RunE and PostRunE would return an error since the execute function can only return single error.

While it is possible to work around this in our code it is certainly not pretty since we need to know the real child command that was executed to call the correct PersistentPostRunE function on that command.
I think this is a problem that should be fixed here in cobra instead, this likely requires the addition of a new option to make this opt in and keep backwards compat.

[GiulianoDecesares]

Same problem here. I'm using the PersistentPostRun to do some cleanup but when the child command returns an error, the PersistentPostRun method is not called and therefore the cleanup is not performed properly. Possible workarounds for this are not pretty at all.

[beatrausch]

I run into the some issue. I have to do some proper cleanup and this is not executed in case run faces an error.

[zeisss]

#914 seems related.

I worked around this by moving cleanup code into cobra.OnFinalize()