v2.4.8 breaks duration default

[binwiederhier]

My urfave/cli version is

v2.4.8

Checklist

• Are you running the latest v2 release? The list of releases is here.
• Did you check the manual for your release? The v2 manual is here
• Did you perform a search about this problem? Here's the Github guide about searching.

Dependency Management

• My project is using go modules.

Describe the bug

The latest version breaks the duration parsing. Here's a sample pipeline that works with 2.4.7 but breaks with 2.4.8: https://github.com/binwiederhier/ntfy/runs/6150219289?check_suite_focus=true

Relevant option here: https://github.com/binwiederhier/ntfy/blob/main/cmd/serve.go#L34

The value for keepalive-interval now defaults to 0 instead of server.DefaultKeepaliveInterval which is 45 * time.Second

To reproduce

https://gist.github.com/binwiederhier/b925ee20822257f55ec04271e40a1437

Observed behavior

c.Duration() returns 0

Expected behavior

c.Duration() returns 45s

Additional context

n/a

Want to fix this yourself?

n/a

Run go version and paste its output here

go version go1.18.1 linux/amd64

[binwiederhier]

This is the output of my own tool (https://github.com/binwiederhier/ntfy) when running with 2.4.8:

$ go run main.go serve
keepalive interval cannot be lower than five seconds
exit status 1

The error comes from serve.go, which reads the value here.

Looking at the diff between 2.4.7 and 2.4.8, I'm pretty sure it's related to the Before:, which is a custom hook defined here.

The diff is small, yet I don't fully understand what's happening. I tried to reproduce it with a minimal example, but so far without success.

[binwiederhier]

I forgot to say: Thank you very much for a fantastic piece of software. I use it in all of my tools! 🥳

[meatballhat]

@binwiederhier Thank you for reporting this! ❤️ 🤘🏼

[binwiederhier]

I found a way to reproduce it:
https://gist.github.com/binwiederhier/b925ee20822257f55ec04271e40a1437

[dearchap]

@binwiederhier Thank you for the detailed bug report. It is amazing. Looks like my "fix" broke this behaviour.
@meatballhat There are 2 ways to fix this, one is to check if a special type of error is returned from the inputSource.XXX(name) to determine if the key doesnt exist otherwise we change the InputSourceContext interface to return 3 elements, (val, err, exists) to determine the next steps to take. Both involve some lengthy changes

[meatballhat]

@dearchap I've got a fix incoming shortly!