implement a custom error type for max transcode attempts

[mjh1]

What does this pull request do? Explain your changes. (required)

Created a custom error and used multi-error wrapping to avoid needing to do any string matching in the error handling.

Specific updates (required)

How did you test each of these updates (required)

I modified the existing unit test.

Does this pull request close any open issues?

Checklist:

• Read the contribution guide
• make runs successfully
• All tests in ./test.sh pass
• README and other documentation updated
• Pending changelog updated

[codecov]

Codecov Report

Merging #2729 (10426d3) into master (a231510) will decrease coverage by 0.36593%.
Report is 5 commits behind head on master.
The diff coverage is 100.00000%.

❗ Current head 10426d3 differs from pull request most recent head 005e441. Consider uploading reports for the commit 005e441 to get more accurate results

@@                 Coverage Diff                 @@
##              master       #2729         +/-   ##
===================================================
- Coverage   56.70668%   56.34075%   -0.36593%     
===================================================
  Files             88          88                 
  Lines          19160       19146         -14     
===================================================
- Hits           10865       10787         -78     
- Misses          7699        7767         +68     
+ Partials         596         592          -4     

Files Changed	Coverage Δ
server/broadcast.go	77.46835% <100.00000%> (-0.08941%)	⬇️

... and 11 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f601912...005e441. Read the comment docs.

Files Changed	Coverage Δ
server/broadcast.go	77.46835% <100.00000%> (-0.08941%)	⬇️

... and 11 files with indirect coverage changes

[cyberj0g]

@mjh1 thanks for looking into that.
I had a slightly more generic solution in mind, like, a wrapper to add more detailed error message along with custom error type, without masking the root error:

ErrFancyCustomError  = errors.New("custom message")
...
func faulty() error {
    err = ...
    return fmt.Errorf("even more detailed message: %w", lperrors.Error(ErrFancyCustomError, err))
    // with a pre-defined message and error type, but root cause error is also included
    // return lperrors.Error(ErrFancyCustomError, err)
}
...
anErr = faulty()
if errors.Is(anErr, ErrFancyCustomError) {
// true
}
// anErr printed like this:
// custom message: even more detailed message: context cancelled

Also, I think it's not a good idea to use errors as a package name, because there's a builtin package with the same name. Maybe I'm inventing the wheel here, let me know what you think.

PS: it's not a high priority feature

[mjh1]

@cyberj0g yeah I was trying to figure out a way to make it generic, it's a little fiddly but I think my latest change should be good enough. It'll get a lot easier in go 1.20 we'll be able wrap multiple errors: https://tip.golang.org/doc/go1.20#errors

[cyberj0g]

https://tip.golang.org/doc/go1.20#errors

Cool! Maybe we could just backport some functions we are interested in, such as: errrors.Is, fmt.Errorf from 1.20 to a lperrors package, and then delete that, once we move go-livepeer to Go 1.20? These functions are few lines usually, it shouldn't be too hard.

[mjh1]

ah good plan 👍

[mjh1]

@cyberj0g annoyingly i'm not sure it's possible, the Is function makes use of this internal reflection package: https://github.com/golang/go/blob/go1.20rc3/src/errors/wrap.go#L8
what do you think?

[cyberj0g]

Ok, let's draft that until Go 1.20 is released. Thanks for researching!

[thomshutt]

Beautiful