Merge lp:~themue/juju-core/go-state-first-error-improvement into lp:~juju/juju-core/trunk

[Frank Mueller]

state: First step of error handling improvement

Error handling in state will be improved to use the new type
StateError containing a describing code, the message and if
wanted a wrapped error. The returned error string depends on
the error code, low-level errors will be covered have to be
retrieved on demand.

ATTENTION: This is a first proposal to discuss this kind of
error wrapping. So not all errors are handled proper.

https://codereview.appspot.com/6306067/

[Gustavo Niemeyer (niemeyer)]

This branch doesn't seem to be addressing the actual issues we have, and
instead introduces some further logic in the state package which we
don't benefit from.

Frank and I brainstormed on it privately, and the conversation was
mailed to juju-dev@. I'm closing this as another branch is coming.

https://codereview.appspot.com/6306067/

Unmerged revisions

214. By Frank Mueller on 2012-06-11

state: First proposal for an improvement of the error handling in state.

213. By Roger Peppe on 2012-06-07

state: store units inside their respective services

Making this change simplifies a lot of code - no need
to keep a separate sequence numbering system for units,
for example.

(original proposal at https://codereview.appspot.com/6247066/)

R=
CC=
https://codereview.appspot.com/6300060

212. By Gustavo Niemeyer on 2012-06-06

.lbox: update to juju-core

211. By Gustavo Niemeyer on 2012-06-06

Translated all paths to use the juju-core project in Launchpad.

210. By Frank Mueller on 2012-06-06

state: Changed relation service mapping in topology.

The first approach mapped roles to service key and relation
name opposite to todays Python implementation. This reduces
the flexibility for future purposes (e.g. multiple peers).
So the mapping has now been changed back from service keys
to relation role and name.

R=
CC=
https://codereview.appspot.com/6268049

209. By Gustavo Niemeyer on 2012-06-06

state: rename Environment method to EnvironConfig

R=TheMue, fwereade
CC=
https://codereview.appspot.com/6295048

208. By William Reade on 2012-06-06

subordinate service units can now be added to principal service units

This entails the following changes:

* new Service.AddUnitSubordinateTo method, which requires a principal unit
* topology.AddUnit now takes an additional principalKey string argument,
  which will be empty for a principal unit;
* new topology.UnitPrincipalKey, which returns the principal unit key for a
  subordinate unit or an error for a principal unit;
* new Unit.IsPrincipal method, which returns whether the unit is a principal
  unit;
* checks to prevent subordinate units being assigned directly to machines.

Most of the diff lines are a result of the change to topology.AddUnit;
sorry for the noise.

R=TheMue, niemeyer
CC=
https://codereview.appspot.com/6268050

207. By Frank Mueller on 2012-06-06

state: Added the method RemoveRelation() to State.

Relation is now an interface defining the single
method relationKey() to allow a function signature
like today in Python. The implementation is now
named relation and can be accessed via type
assertion.

R=niemeyer
CC=
https://codereview.appspot.com/6250076

206. By William Reade on 2012-06-06

trivial: fix inconsistency in deploy command documentation

R=
CC=
https://codereview.appspot.com/6304045

205. By William Reade on 2012-06-06

add AssignmentPolicy to Environ

This is very likely to migrate to a Settings type at some stage, but I don't
think that's justified yet.

R=niemeyer
CC=
https://codereview.appspot.com/6259062

=== modified file 'state/internal_test.go'
--- state/internal_test.go	2012-06-07 11:51:54 +0000
+++ state/internal_test.go	2012-06-11 14:08:22 +0000
@@ -290,7 +290,7 @@
 	err = s.t.AddUnit("unit-0-05", "")
 	c.Assert(err, IsNil)
 	_, err = s.t.UnitPrincipalKey("unit-0-05")
-	c.Assert(err, Equals, unitNotSubordinate)
+	c.Assert(err, Equals, unitNotSubordinateError)
 	err = s.t.AddUnit("unit-0-12", "")
 	c.Assert(err, IsNil)
 	err = s.t.AddUnit("unit-1-07", "unit-0-05")
@@ -515,8 +515,8 @@
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
 		Services: map[string]*topoRelationService{
-			"service-p":     &topoRelationService{RoleProvider, "db"},
-			"illegal": &topoRelationService{RoleRequirer, "db"},
+			"service-p": &topoRelationService{RoleProvider, "db"},
+			"illegal":   &topoRelationService{RoleRequirer, "db"},
 		},
 	})
 	c.Assert(err, ErrorMatches, `service with key "illegal" not found`)
@@ -679,11 +679,11 @@
 
 	// Endpoints without relation.
 	_, err = s.t.RelationKey(mysqlep3, blogep3)
-	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "wordpress:db"`)
+	c.Assert(err, ErrorMatches, `no relation between "mysql:db" and "wordpress:db"`)
 
 	// Mix of endpoints of two relations.
 	_, err = s.t.RelationKey(mysqlep1, blogep2)
-	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "wordpress:db"`)
+	c.Assert(err, ErrorMatches, `no relation between "mysql:db" and "wordpress:db"`)
 
 	// Illegal number of endpoints.
 	_, err = s.t.RelationKey()
@@ -712,13 +712,13 @@
 
 	key, err := s.t.RelationKey(mysqlep1, blogep2)
 	c.Assert(key, Equals, "")
-	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "illegal-wordpress:db"`)
+	c.Assert(err, ErrorMatches, `no relation between "mysql:db" and "illegal-wordpress:db"`)
 	key, err = s.t.RelationKey(mysqlep2, blogep1)
 	c.Assert(key, Equals, "")
-	c.Assert(err, ErrorMatches, `state: no relation between "illegal-mysql:db" and "wordpress:db"`)
+	c.Assert(err, ErrorMatches, `no relation between "illegal-mysql:db" and "wordpress:db"`)
 	key, err = s.t.RelationKey(mysqlep1, riakep3)
 	c.Assert(key, Equals, "")
-	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "riak:ring"`)
+	c.Assert(err, ErrorMatches, `no relation between "mysql:db" and "riak:ring"`)
 }
 
 func (s *TopologySuite) TestPeerRelationKeyEndpoints(c *C) {
@@ -751,7 +751,7 @@
 
 	// Endpoint without relation.
 	key, err = s.t.RelationKey(riakep3)
-	c.Assert(err, ErrorMatches, `state: no peer relation for "riak:ring"`)
+	c.Assert(err, ErrorMatches, `no peer relation for "riak:ring"`)
 }
 
 func (s *TopologySuite) TestPeerRelationKeyIllegalEndpoints(c *C) {
@@ -767,7 +767,7 @@
 
 	key, err := s.t.RelationKey(riakep1)
 	c.Assert(key, Equals, "")
-	c.Assert(err, ErrorMatches, `state: no peer relation for "riak:illegal-ring"`)
+	c.Assert(err, ErrorMatches, `no peer relation for "riak:illegal-ring"`)
 }
 
 type ConfigNodeSuite struct {

=== modified file 'state/machine.go'
--- state/machine.go	2012-06-07 11:51:54 +0000
+++ state/machine.go	2012-06-11 14:08:22 +0000
@@ -35,7 +35,7 @@
 func (m *Machine) WaitAgentAlive(timeout time.Duration) error {
 	err := presence.WaitAlive(m.st.zk, m.zkAgentPath(), timeout)
 	if err != nil {
-		return fmt.Errorf("state: waiting for agent of machine %s: %v", m, err)
+		return fmt.Errorf("waiting for agent of machine %s: %v", m, err)
 	}
 	return nil
 }
@@ -61,7 +61,7 @@
 	if id, ok := v.(string); ok {
 		return id, nil
 	}
-	return "", fmt.Errorf("state: invalid internal machine key type: %T", v)
+	return "", fmt.Errorf("invalid internal machine key type: %T", v)
 }
 
 // SetInstanceId sets the provider specific machine id for this machine.

=== modified file 'state/relation.go'
--- state/relation.go	2012-06-01 10:33:35 +0000
+++ state/relation.go	2012-06-11 14:08:22 +0000
@@ -1,6 +1,8 @@
 package state
 
-import ()
+import (
+	"fmt"
+)
 
 // RelationRole defines the role of a relation endpoint.
 type RelationRole string
@@ -19,6 +21,22 @@
 	ScopeContainer RelationScope = "container"
 )
 
+// NoRelationError represents a relation not found for one or more endpoints.
+type NoRelationError struct {
+	Endpoints []RelationEndpoint
+}
+
+// Error returns the string representation of the error.
+func (e NoRelationError) Error() string {
+	switch len(e.Endpoints) {
+	case 1:
+		return fmt.Sprintf("no peer relation for %q", e.Endpoints[0])
+	case 2:
+		return fmt.Sprintf("no relation between %q and %q", e.Endpoints[0], e.Endpoints[1])
+	}
+	panic("illegal relation")
+}
+
 // RelationEndpoint represents one endpoint of a relation.
 type RelationEndpoint struct {
 	ServiceName   string

=== modified file 'state/service.go'
--- state/service.go	2012-06-07 11:51:54 +0000
+++ state/service.go	2012-06-11 14:08:22 +0000
@@ -86,7 +86,7 @@
 	key := pathPkg.Base(path)
 	addUnit := func(t *topology) error {
 		if !t.HasService(s.key) {
-			return stateChanged
+			return stateChangedError
 		}
 		err := t.AddUnit(key, principalKey)
 		if err != nil {
@@ -140,7 +140,7 @@
 	}
 	removeUnit := func(t *topology) error {
 		if !t.HasUnit(unit.key) {
-			return stateChanged
+			return stateChangedError
 		}
 		if err := t.RemoveUnit(unit.key); err != nil {
 			return err
@@ -168,7 +168,7 @@
 		return nil, err
 	}
 	if !topology.HasService(s.key) {
-		return nil, stateChanged
+		return nil, stateChangedError
 	}
 
 	// Check that unit exists.
@@ -190,7 +190,7 @@
 		return nil, err
 	}
 	if !topology.HasService(s.key) {
-		return nil, stateChanged
+		return nil, stateChangedError
 	}
 	keys, err := topology.UnitKeys(s.key)
 	if err != nil {

=== modified file 'state/state.go'
--- state/state.go	2012-06-07 11:51:54 +0000
+++ state/state.go	2012-06-11 14:08:22 +0000
@@ -46,19 +46,19 @@
 	key := machineKey(id)
 	removeMachine := func(t *topology) error {
 		if !t.HasMachine(key) {
-			return fmt.Errorf("machine not found")
+			return stateError(MachineError, nil, "machine not found")
 		}
 		hasUnits, err := t.MachineHasUnits(key)
 		if err != nil {
 			return err
 		}
 		if hasUnits {
-			return fmt.Errorf("machine has units")
+			return stateError(MachineError, nil, "machine has units")
 		}
 		return t.RemoveMachine(key)
 	}
 	if err := retryTopologyChange(s.zk, removeMachine); err != nil {
-		return fmt.Errorf("can't remove machine %d: %v", id, err)
+		return stateError(MachineError, err, "can't remove machine %d", id)
 	}
 	return zkRemoveTree(s.zk, fmt.Sprintf("/machines/%s", key))
 }
@@ -207,7 +207,7 @@
 	// Remove the service from the topology.
 	removeService := func(t *topology) error {
 		if !t.HasService(svc.key) {
-			return stateChanged
+			return stateChangedError
 		}
 		t.RemoveService(svc.key)
 		return nil
@@ -381,13 +381,10 @@
 // RemoveRelation removes the relation.
 func (s *State) RemoveRelation(relation *Relation) error {
 	removeRelation := func(t *topology) error {
-		_, err := t.Relation(relation.key)
-		if err != nil {
-			return fmt.Errorf("can't remove relation: %v", err)
-		}
 		return t.RemoveRelation(relation.key)
 	}
-	// TODO: Improve high-level errors, no passing of low-level 
-	// errors directly to the caller.
-	return retryTopologyChange(s.zk, removeRelation)
+	if err := retryTopologyChange(s.zk, removeRelation); err != nil {
+		return stateError(RelationError, err, "can't remove relation")
+	}
+	return nil
 }

=== modified file 'state/state_test.go'
--- state/state_test.go	2012-06-07 11:51:54 +0000
+++ state/state_test.go	2012-06-11 14:08:22 +0000
@@ -221,7 +221,7 @@
 	c.Assert(err, IsNil)
 
 	id, err := machine.InstanceId()
-	c.Assert(err, ErrorMatches, "state: invalid internal machine key type: .*")
+	c.Assert(err, ErrorMatches, "invalid internal machine key type: .*")
 	c.Assert(id, Equals, "")
 }
 
@@ -305,7 +305,7 @@
 	c.Assert(alive, Equals, false)
 
 	err = machine0.WaitAgentAlive(timeout)
-	c.Assert(err, ErrorMatches, `state: waiting for agent of machine 0: presence: still not alive after timeout`)
+	c.Assert(err, ErrorMatches, `waiting for agent of machine 0: presence: still not alive after timeout`)
 
 	pinger, err := machine0.SetAgentAlive()
 	c.Assert(err, IsNil)
@@ -675,7 +675,7 @@
 
 	// Assigning the unit to a different machine should fail.
 	err = unit.AssignToMachine(machineTwo)
-	c.Assert(err, ErrorMatches, `unit "wordpress/0" already assigned to machine 0`)
+	c.Assert(err, ErrorMatches, `can't assign unit "wordpress/0" to machine 1: unit "wordpress/0" already assigned to machine 0`)
 
 	machineId, err := unit.AssignedMachineId()
 	c.Assert(err, IsNil)
@@ -1249,5 +1249,5 @@
 	c.Assert(hasRelation, Equals, false)
 	c.Assert(err, ErrorMatches, `relation "relation-0000000000" does not exist`)
 	err = s.st.RemoveRelation(relation)
-	c.Assert(err, ErrorMatches, `can't remove relation: relation "relation-0000000000" does not exist`)
+	c.Assert(err, ErrorMatches, `can't remove relation`)
 }

=== modified file 'state/topology.go'
--- state/topology.go	2012-06-07 11:51:54 +0000
+++ state/topology.go	2012-06-11 14:08:22 +0000
@@ -1,7 +1,6 @@
 package state
 
 import (
-	"errors"
 	"fmt"
 	"launchpad.net/goyaml"
 	"launchpad.net/gozk/zookeeper"
@@ -13,22 +12,6 @@
 // when we know that a version is in fact actually incompatible.
 const topologyVersion = 1
 
-// NoRelationError represents a relation not found for one or more endpoints.
-type NoRelationError struct {
-	Endpoints []RelationEndpoint
-}
-
-// Error returns the string representation of the error.
-func (e NoRelationError) Error() string {
-	switch len(e.Endpoints) {
-	case 1:
-		return fmt.Sprintf("state: no peer relation for %q", e.Endpoints[0])
-	case 2:
-		return fmt.Sprintf("state: no relation between %q and %q", e.Endpoints[0], e.Endpoints[1])
-	}
-	panic("state: illegal relation")
-}
-
 // topoTopology is used to marshal and unmarshal the content
 // of the /topology node in ZooKeeper.
 type topoTopology struct {
@@ -76,10 +59,10 @@
 // check verifies that r is a proper relation.
 func (r *topoRelation) check() error {
 	if len(r.Interface) == 0 {
-		return fmt.Errorf("relation interface is empty")
+		return stateError(TopologyError, nil, "relation interface is empty")
 	}
 	if len(r.Services) == 0 {
-		return fmt.Errorf("relation has no services")
+		return stateError(TopologyError, nil, "relation has no services")
 	}
 	counterpart := map[RelationRole]RelationRole{
 		RoleRequirer: RoleProvider,
@@ -88,21 +71,21 @@
 	}
 	for serviceKey, service := range r.Services {
 		if serviceKey == "" {
-			return fmt.Errorf("relation has service with empty key")
+			return stateError(TopologyError, nil, "relation has service with empty key")
 		}
 		if service.RelationName == "" {
-			return fmt.Errorf("relation has %s service with empty relation name", service.RelationRole)
+			return stateError(TopologyError, nil, "relation has %s service with empty relation name", service.RelationRole)
 		}
 		counterRole, ok := counterpart[service.RelationRole]
 		if !ok {
-			return fmt.Errorf("relation has unknown service role: %q", service.RelationRole)
+			return stateError(TopologyError, nil, "relation has unknown service role: %q", service.RelationRole)
 		}
 		if !r.hasServiceWithRole(counterRole) {
-			return fmt.Errorf("relation has %s but no %s", service.RelationRole, counterRole)
+			return stateError(TopologyError, nil, "relation has %s but no %s", service.RelationRole, counterRole)
 		}
 	}
 	if len(r.Services) > 2 {
-		return fmt.Errorf("relation with mixed peer, provider, and requirer roles")
+		return stateError(TopologyError, nil, "relation with mixed peer, provider, and requirer roles")
 	}
 	return nil
 }
@@ -132,7 +115,7 @@
 			// No topology node, so return empty topology.
 			return parseTopology("")
 		}
-		return nil, err
+		return nil, stateError(PersistencyError, err, "can't read topology")
 	}
 	return parseTopology(yaml)
 }
@@ -141,7 +124,7 @@
 func (t *topology) dump() (string, error) {
 	yaml, err := goyaml.Marshal(t.topology)
 	if err != nil {
-		return "", err
+		return "", stateError(DataFormatError, err, "illegal topology format")
 	}
 	return string(yaml), nil
 }
@@ -156,7 +139,7 @@
 	if t.topology.Machines == nil {
 		t.topology.Machines = make(map[string]*topoMachine)
 	} else if t.HasMachine(key) {
-		return fmt.Errorf("attempted to add duplicated machine %q", key)
+		return stateError(TopologyError, nil, "attempted to add duplicated machine %q", key)
 	}
 	t.topology.Machines[key] = &topoMachine{}
 	return nil
@@ -169,7 +152,7 @@
 		return err
 	}
 	if ok {
-		return fmt.Errorf("can't remove machine %q while units ared assigned", key)
+		return stateError(TopologyError, nil, "can't remove machine %q while units ared assigned", key)
 	}
 	// Machine exists and has no units, so remove it.
 	delete(t.topology.Machines, key)
@@ -213,10 +196,10 @@
 		t.topology.Services = make(map[string]*topoService)
 	}
 	if t.HasService(key) {
-		return fmt.Errorf("attempted to add duplicated service %q", key)
+		return stateError(TopologyError, nil, "attempted to add duplicated service %q", key)
 	}
 	if _, err := t.ServiceKey(name); err == nil {
-		return fmt.Errorf("service name %q already in use", name)
+		return stateError(TopologyError, nil, "service name %q already in use", name)
 	}
 	t.topology.Services[key] = &topoService{
 		Name:  name,
@@ -235,7 +218,7 @@
 		return err
 	}
 	if len(relations) > 0 {
-		return fmt.Errorf("cannot remove service %q with active relations", key)
+		return stateError(TopologyError, nil, "cannot remove service %q with active relations", key)
 	}
 	delete(t.topology.Services, key)
 	return nil
@@ -253,7 +236,7 @@
 			return key, nil
 		}
 	}
-	return "", fmt.Errorf("service with name %q not found", name)
+	return "", stateError(TopologyError, nil, "service with name %q not found", name)
 }
 
 // ServiceKeys returns all service keys.
@@ -271,7 +254,7 @@
 	if svc, ok := t.topology.Services[key]; ok {
 		return svc.Name, nil
 	}
-	return "", fmt.Errorf("service with key %q not found", key)
+	return "", stateError(TopologyError, nil, "service with key %q not found", key)
 }
 
 // HasUnit returns true if a unit with given service and unit keys exists.
@@ -291,7 +274,7 @@
 		return err
 	}
 	if _, ok := svc.Units[unitKey]; ok {
-		return fmt.Errorf("unit %q already in use", unitKey)
+		return stateError(TopologyError, nil, "unit %q already in use", unitKey)
 	}
 	svc.Units[unitKey] = &topoUnit{Principal: principalKey}
 	return nil
@@ -332,35 +315,35 @@
 	return fmt.Sprintf("%s/%d", svc.Name, keySeq(unitKey)), nil
 }
 
-// unitNotSubordinate indicates that a unit is principal rather than subordinate.
-var unitNotSubordinate = errors.New("service unit is a principal rather than a subordinate")
+// unitNotSubordinateError indicates that a unit is principal rather than subordinate.
+var unitNotSubordinateError = stateError(TopologyError, nil, "service unit is a principal rather than a subordinate")
 
 // UnitPrincipalKey returns the unit key of the principal unit alongside which
 // the specified subordinate unit is deployed. If the specified unit is not
-// subordinate, unitNotSubordinate will be returned.
+// subordinate, unitNotSubordinateError will be returned.
 func (t *topology) UnitPrincipalKey(unitKey string) (string, error) {
 	_, unit, err := t.serviceAndUnit(unitKey)
 	if err != nil {
 		return "", err
 	}
 	if unit.Principal == "" {
-		return "", unitNotSubordinate
+		return "", unitNotSubordinateError
 	}
 	return unit.Principal, nil
 }
 
-// unitNotAssigned indicates that a unit is not assigned to a machine.
-var unitNotAssigned = errors.New("unit not assigned to machine")
+// unitNotAssignedError indicates that a unit is not assigned to a machine.
+var unitNotAssignedError = stateError(TopologyError, nil, "unit not assigned to machine")
 
 // UnitMachineKey returns the key of an assigned machine of the unit. If no machine
-// is assigned the error unitNotAssigned will be returned.
+// is assigned the error unitNotAssignedError will be returned.
 func (t *topology) UnitMachineKey(unitKey string) (string, error) {
 	_, unit, err := t.serviceAndUnit(unitKey)
 	if err != nil {
 		return "", err
 	}
 	if unit.Machine == "" {
-		return "", unitNotAssigned
+		return "", unitNotAssignedError
 	}
 	return unit.Machine, nil
 }
@@ -378,10 +361,10 @@
 		return err
 	}
 	if unit.Principal != "" {
-		return errors.New("cannot assign subordinate units directly to machines")
+		return stateError(TopologyError, nil, "cannot assign subordinate units directly to machines")
 	}
 	if unit.Machine != "" {
-		return fmt.Errorf("unit %q already assigned to machine %q", unitKey, unit.Machine)
+		return stateError(TopologyError, nil, "unit %q already assigned to machine %q", unitKey, unit.Machine)
 	}
 	unit.Machine = machineKey
 	return nil
@@ -394,7 +377,7 @@
 		return err
 	}
 	if unit.Machine == "" {
-		return fmt.Errorf("unit %q not assigned to a machine", unitKey)
+		return stateError(TopologyError, nil, "unit %q not assigned to a machine", unitKey)
 	}
 	unit.Machine = ""
 	return nil
@@ -403,7 +386,7 @@
 // Relation returns the relation with key from the topology.
 func (t *topology) Relation(key string) (*topoRelation, error) {
 	if t.topology.Relations == nil || t.topology.Relations[key] == nil {
-		return nil, fmt.Errorf("relation %q does not exist", key)
+		return nil, stateError(TopologyError, nil, "relation %q does not exist", key)
 	}
 	return t.topology.Relations[key], nil
 }
@@ -415,7 +398,7 @@
 	}
 	_, ok := t.topology.Relations[relationKey]
 	if ok {
-		return fmt.Errorf("relation key %q already in use", relationKey)
+		return stateError(TopologyError, nil, "relation key %q already in use", relationKey)
 	}
 	// Check if the relation definition and the service keys are valid.
 	if err := relation.check(); err != nil {
@@ -514,7 +497,7 @@
 	if m, ok := t.topology.Machines[machineKey]; ok {
 		return m, nil
 	}
-	return nil, fmt.Errorf("machine with key %q not found", machineKey)
+	return nil, stateError(TopologyError, nil, "machine with key %q not found", machineKey)
 }
 
 // service returns the service for the given key.
@@ -522,7 +505,7 @@
 	if svc, ok := t.topology.Services[serviceKey]; ok {
 		return svc, nil
 	}
-	return nil, fmt.Errorf("service with key %q not found", serviceKey)
+	return nil, stateError(TopologyError, nil, "service with key %q not found", serviceKey)
 }
 
 // serviceAndUnit returns the service and unit for the given unit key.
@@ -538,7 +521,7 @@
 	if unit, ok := svc.Units[unitKey]; ok {
 		return svc, unit, nil
 	}
-	return nil, nil, fmt.Errorf("unit with key %q not found", unitKey)
+	return nil, nil, stateError(TopologyError, nil, "unit with key %q not found", unitKey)
 }
 
 // relation returns the relation for the given key
@@ -546,17 +529,17 @@
 	if t, ok := t.topology.Relations[relationKey]; ok {
 		return t, nil
 	}
-	return nil, fmt.Errorf("relation with key %q not found", relationKey)
+	return nil, stateError(TopologyError, nil, "relation with key %q not found", relationKey)
 }
 
 // parseTopology returns the topology represented by yaml.
 func parseTopology(yaml string) (*topology, error) {
 	t := &topology{topology: &topoTopology{Version: topologyVersion}}
 	if err := goyaml.Unmarshal([]byte(yaml), t.topology); err != nil {
-		return nil, err
+		return nil, stateError(DataFormatError, err, "illegal topology format")
 	}
 	if t.topology.Version != topologyVersion {
-		return nil, fmt.Errorf("incompatible topology versions: got %d, want %d",
+		return nil, stateError(TopologyError, nil, "incompatible topology versions: got %d, want %d",
 			t.topology.Version, topologyVersion)
 	}
 	return t, nil
@@ -583,5 +566,11 @@
 		}
 		return it.dump()
 	}
-	return zk.RetryChange(zkTopologyPath, 0, zkPermAll, change)
+	if err := zk.RetryChange(zkTopologyPath, 0, zkPermAll, change); err != nil {
+		if sterr, ok := err.(StateError); ok {
+			return sterr
+		}
+		return stateError(PersistencyError, err, "can't change topology")
+	}
+	return nil
 }

=== modified file 'state/unit.go'
--- state/unit.go	2012-06-07 12:06:16 +0000
+++ state/unit.go	2012-06-11 14:08:22 +0000
@@ -194,7 +194,7 @@
 		return false, err
 	}
 	_, err = topology.UnitPrincipalKey(u.key)
-	if err == unitNotSubordinate {
+	if err == unitNotSubordinateError {
 		return true, nil
 	}
 	return false, err
@@ -216,7 +216,7 @@
 			return
 		}
 	case AssignUnused:
-		if _, err = u.AssignToUnusedMachine(); err != noUnusedMachines {
+		if _, err = u.AssignToUnusedMachine(); err != noUnusedMachinesError {
 			return
 		}
 		if m, err = u.st.AddMachine(); err != nil {
@@ -235,7 +235,7 @@
 		return 0, err
 	}
 	if !topology.HasUnit(u.key) {
-		return 0, stateChanged
+		return 0, stateChangedError
 	}
 	machineKey, err := topology.UnitMachineKey(u.key)
 	if err != nil {
@@ -248,10 +248,10 @@
 func (u *Unit) AssignToMachine(machine *Machine) error {
 	assignUnit := func(t *topology) error {
 		if !t.HasUnit(u.key) {
-			return stateChanged
+			return stateChangedError
 		}
 		machineKey, err := t.UnitMachineKey(u.key)
-		if err == unitNotAssigned {
+		if err == unitNotAssignedError {
 			return t.AssignUnitToMachine(u.key, machine.key)
 		} else if err != nil {
 			return err
@@ -259,12 +259,17 @@
 			// Everything is fine, it's already assigned.
 			return nil
 		}
-		return fmt.Errorf("unit %q already assigned to machine %d", u.Name(), keySeq(machineKey))
-	}
-	return retryTopologyChange(u.st.zk, assignUnit)
+		return stateError(UnitError, nil, "unit %q already assigned to machine %d", u.Name(), keySeq(machineKey))
+	}
+	if err := retryTopologyChange(u.st.zk, assignUnit); err != nil {
+		return stateError(UnitError, err, "can't assign unit %q to machine %d", u.Name(), machine.Id())
+	}
+	return nil
 }
 
-var noUnusedMachines = errors.New("no unused machine found")
+// noUnusedMachinesError is returned if no unused machine for the 
+// assignment of a unit can be found.
+var noUnusedMachinesError = stateError(UnitError, nil, "no unused machine found")
 
 // AssignToUnusedMachine assigns u to a machine without other units.
 // If there are no unused machines besides machine 0, an error is returned.
@@ -272,7 +277,7 @@
 	machineKey := ""
 	assignUnusedUnit := func(t *topology) error {
 		if !t.HasUnit(u.key) {
-			return stateChanged
+			return stateChangedError
 		}
 		for _, machineKey = range t.MachineKeys() {
 			if keySeq(machineKey) != 0 {
@@ -288,7 +293,7 @@
 			machineKey = ""
 		}
 		if machineKey == "" {
-			return noUnusedMachines
+			return noUnusedMachinesError
 		}
 		if err := t.AssignUnitToMachine(u.key, machineKey); err != nil {
 			return err
@@ -306,7 +311,7 @@
 func (u *Unit) UnassignFromMachine() error {
 	unassignUnit := func(t *topology) error {
 		if !t.HasUnit(u.key) {
-			return stateChanged
+			return stateChangedError
 		}
 		// If for whatever reason it's already not assigned to a
 		// machine, ignore it and move forward so that we don't

=== modified file 'state/util.go'
--- state/util.go	2012-06-06 10:50:44 +0000
+++ state/util.go	2012-06-11 14:08:22 +0000
@@ -5,7 +5,6 @@
 package state
 
 import (
-	"errors"
 	"fmt"
 	"launchpad.net/goyaml"
 	"launchpad.net/gozk/zookeeper"
@@ -13,9 +12,51 @@
 	"sort"
 )
 
+// ErrorCode represents a kind of state error.
+type ErrorCode int
+
+const (
+	PersistencyError  ErrorCode = -4
+	DataFormatError   ErrorCode = -3
+	StateChangedError ErrorCode = -2
+	TopologyError     ErrorCode = -1
+	OK                ErrorCode = 0
+	MachineError      ErrorCode = 1
+	UnitError         ErrorCode = 2
+	RelationError     ErrorCode = 3
+)
+
+// StateError encapsulates errors occuring during topology operations.
+type StateError struct {
+	Code    ErrorCode
+	Message string
+	Err     error
+}
+
+// stateError creates a new state error.
+func stateError(code ErrorCode, err error, msg string, args ...interface{}) error {
+	return StateError{
+		Code:    code,
+		Message: fmt.Sprintf(msg, args...),
+		Err:     err,
+	}
+}
+
+// Error returns the string representation of the error.
+func (e StateError) Error() string {
+	if e.Err != nil {
+		if sterr, ok := e.Err.(StateError); ok {
+			if sterr.Code > OK {
+				return e.Message + ": " + e.Err.Error()
+			}
+		}
+	}
+	return e.Message
+}
+
 var (
-	// stateChange is a common error inside the state processing.
-	stateChanged = errors.New("environment state has changed")
+	// stateChangedError is a common error inside the state processing.
+	stateChangedError = stateError(StateChangedError, nil, "environment state has changed")
 	// zkPermAll is a convenience variable for creating new nodes.
 	zkPermAll = zookeeper.WorldACL(zookeeper.PERM_ALL)
 )
@@ -89,7 +130,7 @@
 	c.cache = copyCache(values)
 	_, err := c.Write()
 	if err != nil {
-		return nil, err
+		return nil, stateError(PersistencyError, err, "can't create config node")
 	}
 	return c, nil
 }
@@ -108,7 +149,7 @@
 // It does not affect the user-set contents.
 func (c *ConfigNode) setPristineContent(content string) error {
 	if err := goyaml.Unmarshal([]byte(content), &c.pristineCache); err != nil {
-		return err
+		return stateError(DataFormatError, err, "illegal content in config node")
 	}
 	return nil
 }
@@ -127,7 +168,7 @@
 	yaml, _, err := c.zk.Get(c.path)
 	if err != nil {
 		if !zookeeper.IsError(err, zookeeper.ZNONODE) {
-			return err
+			return stateError(PersistencyError, err, "can't read config node")
 		}
 	}
 	if err := c.setPristineContent(yaml); err != nil {
@@ -153,7 +194,7 @@
 		current := make(map[string]interface{})
 		if yaml != "" {
 			if err := goyaml.Unmarshal([]byte(yaml), current); err != nil {
-				return "", err
+				return "", stateError(DataFormatError, err, "illegal content in config node")
 			}
 		}
 		for key := range cacheKeys(c.pristineCache, c.cache) {
@@ -184,12 +225,15 @@
 		}
 		currentYaml, err := goyaml.Marshal(current)
 		if err != nil {
-			return "", err
+			return "", stateError(DataFormatError, err, "illegal content for config node")
 		}
 		return string(currentYaml), nil
 	}
 	if err := c.zk.RetryChange(c.path, 0, zkPermAll, applyChanges); err != nil {
-		return nil, err
+		if sterr, ok := err.(StateError); ok {
+			return nil, sterr
+		}
+		return nil, stateError(PersistencyError, err, "can't change config node")
 	}
 	sort.Sort(itemChangeSlice(changes))
 	c.pristineCache = copyCache(c.cache)
@@ -249,7 +293,7 @@
 		if zookeeper.IsError(err, zookeeper.ZNONODE) {
 			return nil
 		}
-		return err
+		return stateError(PersistencyError, err, "can't remove tree")
 	}
 	for _, child := range children {
 		if err := zkRemoveTree(zk, pathpkg.Join(path, child)); err != nil {
@@ -262,7 +306,7 @@
 	}
 	err = zk.Delete(path, -1)
 	if err != nil && !zookeeper.IsError(err, zookeeper.ZNONODE) {
-		return err
+		return stateError(PersistencyError, err, "can't remove tree")
 	}
 	return nil
 }

=== modified file 'state/watcher_test.go'
--- state/watcher_test.go	2012-06-06 19:19:11 +0000
+++ state/watcher_test.go	2012-06-11 14:08:22 +0000
@@ -95,7 +95,7 @@
 	}
 
 	err = configWatcher.Stop()
-	c.Assert(err, ErrorMatches, "YAML error: .*")
+	c.Assert(err, ErrorMatches, "illegal content in config node")
 }
 
 type unitWatchNeedsUpgradeTest struct {