Skip to content

Commit

Permalink
Merge pull request #23 from tlm/errors-upgrade
Browse files Browse the repository at this point in the history
Upgrade to latest juju/errors package.
  • Loading branch information
hpidcock authored Nov 2, 2022
2 parents e23db69 + 7bc5a20 commit f7761ab
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 114 deletions.
6 changes: 3 additions & 3 deletions catacomb/catacomb.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ func (catacomb *Catacomb) Kill(err error) {
if err == tomb.ErrDying {
err = errors.New("bad catacomb Kill: tomb.ErrDying")
}
cause := errors.Cause(err)
if match, ok := cause.(dyingError); ok {
if catacomb != match.catacomb {
var dyingErr dyingError
if errors.As(err, &dyingErr) {
if catacomb != dyingErr.catacomb {
err = errors.Errorf("bad catacomb Kill: other catacomb's ErrDying")
} else {
err = tomb.ErrDying
Expand Down
10 changes: 4 additions & 6 deletions catacomb/catacomb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (s *CatacombSuite) TestAddWhenDyingReturnsWorkerError(c *gc.C) {
s.fix.catacomb.Kill(nil)

actual := s.fix.catacomb.Add(w)
c.Assert(errors.Cause(actual), gc.Equals, expect)
c.Assert(errors.Is(actual, expect), jc.IsTrue)
w.assertDead(c)
})
c.Check(err, jc.ErrorIsNil)
Expand All @@ -267,7 +267,7 @@ func (s *CatacombSuite) TestAddWhenDeadReturnsWorkerError(c *gc.C) {
expect := errors.New("squelch")
w := s.fix.startErrorWorker(c, expect)
actual := s.fix.catacomb.Add(w)
c.Assert(errors.Cause(actual), gc.Equals, expect)
c.Assert(errors.Is(actual, expect), jc.IsTrue)
w.assertDead(c)
}

Expand Down Expand Up @@ -394,8 +394,7 @@ func (s *CatacombSuite) TestStressAddKillRaces(c *gc.C) {
// the important thing is that it already exists so we can hit
// Add() as soon as possible, just like the Kill() below.
if err := s.fix.catacomb.Add(w); err != nil {
cause := errors.Cause(err)
c.Check(cause, gc.Equals, errFailed)
c.Check(errors.Is(err, errFailed), jc.IsTrue)
}
})
together(func() {
Expand All @@ -407,8 +406,7 @@ func (s *CatacombSuite) TestStressAddKillRaces(c *gc.C) {
close(block)
wg.Wait()
})
cause := errors.Cause(err)
c.Check(cause, gc.Equals, errFailed)
c.Check(errors.Is(err, errFailed), jc.IsTrue)
}

func (s *CatacombSuite) TestReusedCatacomb(c *gc.C) {
Expand Down
28 changes: 15 additions & 13 deletions dependency/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,9 @@ func (engine *Engine) context(name string, inputs []string, abort <-chan struct{
}
}

var errAborted = errors.New("aborted before delay elapsed")
const (
errAborted = errors.ConstError("aborted before delay elapsed")
)

// runWorker starts the supplied manifold's worker and communicates it back to the
// loop goroutine; waits for worker completion; and communicates any error encountered
Expand Down Expand Up @@ -520,10 +522,10 @@ func (engine *Engine) runWorker(name string, delay time.Duration, start StartFun

startWorkerAndWait := func() error {
worker, err := startAfterDelay()
switch errors.Cause(err) {
case errAborted:
switch {
case errors.Is(err, errAborted):
return errAborted
case nil:
case err == nil:
engine.config.Logger.Tracef("running %q manifold worker", name)
default:
return err
Expand Down Expand Up @@ -596,18 +598,18 @@ func (engine *Engine) gotStopped(name string, err error, resourceLog []resourceA
// Copy current info and check for reasons to stop the engine.
info := engine.current[name]

switch errors.Cause(err) {
case nil:
switch {
case err == nil:
engine.config.Logger.Debugf("%q manifold worker completed successfully", name)
info.recentErrors = 0
case errAborted:
case errors.Is(err, errAborted):
// The start attempt was aborted, so we haven't really started.
engine.config.Logger.Tracef("%q manifold worker bounced while starting", name)
// If we have been aborted while trying to start, we are more likely
// to be able to start, so reset the start attempts.
info.startAttempts = 0
info.recentErrors = 1
case ErrMissing:
case errors.Is(err, ErrMissing):
engine.config.Logger.Tracef("%q manifold worker failed to start: %v", name, err)
// missing a dependency does (not?) trigger exponential backoff
info.recentErrors = 1
Expand Down Expand Up @@ -661,19 +663,19 @@ func (engine *Engine) gotStopped(name string, err error, resourceLog []resourceA
engine.requestStart(name, engine.config.BounceDelay)
} else {
// If we didn't stop it ourselves, we need to interpret the error.
switch errors.Cause(err) {
case nil, errAborted:
switch {
case err == nil, errors.Is(err, errAborted):
// Nothing went wrong; the task completed successfully. Nothing
// needs to be done (unless the inputs change, in which case it
// gets to check again).
case ErrMissing:
case errors.Is(err, ErrMissing):
// The task can't even start with the current state. Nothing more
// can be done (until the inputs change, in which case we retry
// anyway).
case ErrBounce:
case errors.Is(err, ErrBounce):
// The task exited but wanted to restart immediately.
engine.requestStart(name, engine.config.BounceDelay)
case ErrUninstall:
case errors.Is(err, ErrUninstall):
// The task should never run again, and can be removed completely.
engine.uninstall(name)
default:
Expand Down
4 changes: 2 additions & 2 deletions dependency/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func (s *EngineSuite) TestStartGetUndeclaredName(c *gc.C) {
err = engine.Install("other-task", dependency.Manifold{
Start: func(context dependency.Context) (worker.Worker, error) {
err := context.Get("some-task", nil)
c.Check(errors.Cause(err), gc.Equals, dependency.ErrMissing)
c.Check(errors.Is(err, dependency.ErrMissing), jc.IsTrue)
c.Check(err, gc.ErrorMatches, `"some-task" not declared: dependency not available`)
close(done)
// Return a real worker so we don't keep restarting and potentially double-closing.
Expand Down Expand Up @@ -665,7 +665,7 @@ func (s *EngineSuite) TestWorstError(c *gc.C) {
mh2.InjectError(c, errors.New("pong"))

err = workertest.CheckKilled(c, engine)
c.Check(errors.Cause(err), gc.Equals, worstErr)
c.Check(errors.Is(err, worstErr), jc.IsTrue)
c.Check(callCount, gc.Equals, 2)
})
}
Expand Down
36 changes: 19 additions & 17 deletions dependency/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,23 +111,6 @@ type Context interface {
// not be called again until its declared inputs change.
type StartFunc func(context Context) (worker.Worker, error)

// ErrMissing can be returned by a StartFunc or a worker to indicate to
// the engine that it can't be usefully restarted until at least one of its
// dependencies changes. There's no way to specify *which* dependency you need,
// because that's a lot of implementation hassle for little practical gain.
var ErrMissing = errors.New("dependency not available")

// ErrBounce can be returned by a StartFunc or a worker to indicate to
// the engine that it should be restarted immediately, instead of
// waiting for ErrorDelay. This is useful for workers which restart
// themselves to alert dependents that an output has changed.
var ErrBounce = errors.New("restart immediately")

// ErrUninstall can be returned by a StartFunc or a worker to indicate to the
// engine that it can/should never run again, and that the originating manifold
// should be completely removed.
var ErrUninstall = errors.New("resource permanently unavailable")

// FilterFunc is an error conversion function for errors returned from workers
// or StartFuncs.
type FilterFunc func(error) error
Expand All @@ -145,3 +128,22 @@ type IsFatalFunc func(err error) bool
// WorstErrorFunc is used to rank fatal errors, to allow an Engine to return the
// single most important error it's encountered.
type WorstErrorFunc func(err0, err1 error) error

const (
// ErrBounce can be returned by a StartFunc or a worker to indicate to
// the engine that it should be restarted immediately, instead of
// waiting for ErrorDelay. This is useful for workers which restart
// themselves to alert dependents that an output has changed.
ErrBounce = errors.ConstError("restart immediately")

// ErrMissing can be returned by a StartFunc or a worker to indicate to
// the engine that it can't be usefully restarted until at least one of its
// dependencies changes. There's no way to specify *which* dependency you need,
// because that's a lot of implementation hassle for little practical gain.
ErrMissing = errors.ConstError("dependency not available")

// ErrUninstall can be returned by a StartFunc or a worker to indicate to the
// engine that it can/should never run again, and that the originating manifold
// should be completely removed.
ErrUninstall = errors.ConstError("resource permanently unavailable")
)
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
module github.com/juju/worker/v3

go 1.17
go 1.19

require (
github.com/juju/clock v0.0.0-20220203021603-d9deb868a28a
github.com/juju/collections v0.0.0-20220203020748-febd7cad8a7a
github.com/juju/errors v0.0.0-20220203013757-bd733f3c86b9
github.com/juju/errors v1.0.0
github.com/juju/loggo v0.0.0-20210728185423-eebad3a902c4
github.com/juju/testing v0.0.0-20220203020004-a0ff61f03494
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
Expand Down
Loading

0 comments on commit f7761ab

Please sign in to comment.