Skip to content

Commit

Permalink
chore: simplified configuration error
Browse files Browse the repository at this point in the history
Renamed ConfigurationCreationError to just configurationError.
Also, removed configuration error from some parts since the
configuration error does not provide additional context; in fact
it often just repeats the same information in the error output.
  • Loading branch information
meling committed Mar 23, 2024
1 parent b681f9e commit 907a8e7
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 28 deletions.
1 change: 1 addition & 0 deletions .vscode/gorums.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Meland's
Meling
memprofile
multicast
naddr
obj's
Paxos
Pedersen
Expand Down
6 changes: 1 addition & 5 deletions config.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package gorums

import (
"fmt"
)

// RawConfiguration represents a static set of nodes on which quorum calls may be invoked.
//
// NOTE: mutating the configuration is not supported.
Expand All @@ -18,7 +14,7 @@ type RawConfiguration []*RawNode
// using the And, WithNewNodes, Except, and WithoutNodes methods.
func NewRawConfiguration(mgr *RawManager, opt NodeListOption) (nodes RawConfiguration, err error) {
if opt == nil {
return nil, ConfigCreationError(fmt.Errorf("missing required node list"))
return nil, configurationError("missing required node list")
}
return opt.newConfig(mgr)
}
Expand Down
22 changes: 10 additions & 12 deletions config_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,18 @@ type nodeIDMap struct {

func (o nodeIDMap) newConfig(mgr *RawManager) (nodes RawConfiguration, err error) {
if len(o.idMap) == 0 {
return nil, ConfigCreationError(fmt.Errorf("node-to-ID map required: WithNodeMap"))
return nil, configurationError("missing required node map")
}
nodes = make(RawConfiguration, 0, len(o.idMap))
for naddr, id := range o.idMap {
node, found := mgr.Node(id)
if !found {
node, err = NewRawNodeWithID(naddr, id)
if err != nil {
return nil, ConfigCreationError(err)
return nil, err
}
err = mgr.AddNode(node)
if err != nil {
return nil, ConfigCreationError(err)
if err = mgr.AddNode(node); err != nil {
return nil, err
}
}
nodes = append(nodes, node)
Expand All @@ -52,18 +51,17 @@ type nodeList struct {

func (o nodeList) newConfig(mgr *RawManager) (nodes RawConfiguration, err error) {
if len(o.addrsList) == 0 {
return nil, ConfigCreationError(fmt.Errorf("node addresses required: WithNodeList"))
return nil, configurationError("missing required node addresses")
}
nodes = make(RawConfiguration, 0, len(o.addrsList))
for _, naddr := range o.addrsList {
node, err := NewRawNode(naddr)
if err != nil {
return nil, ConfigCreationError(err)
return nil, err
}
if n, found := mgr.Node(node.ID()); !found {
err = mgr.AddNode(node)
if err != nil {
return nil, ConfigCreationError(err)
if err = mgr.AddNode(node); err != nil {
return nil, err
}
} else {
node = n
Expand All @@ -88,14 +86,14 @@ type nodeIDs struct {

func (o nodeIDs) newConfig(mgr *RawManager) (nodes RawConfiguration, err error) {
if len(o.nodeIDs) == 0 {
return nil, ConfigCreationError(fmt.Errorf("node IDs required: WithNodeIDs"))
return nil, configurationError("missing required node IDs")
}
nodes = make(RawConfiguration, 0, len(o.nodeIDs))
for _, id := range o.nodeIDs {
node, found := mgr.Node(id)
if !found {
// Node IDs must have been registered previously
return nil, ConfigCreationError(fmt.Errorf("node ID %d not found", id))
return nil, configurationError(fmt.Sprintf("node %d not found", id))
}
nodes = append(nodes, node)
}
Expand Down
7 changes: 3 additions & 4 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ import (
"fmt"
)

// ConfigCreationError returns an error reporting that a Configuration
// could not be created due to err.
func ConfigCreationError(err error) error {
return fmt.Errorf("could not create configuration: %s", err.Error())
// configurationError reports that a Configuration could not be created.
func configurationError(desc string) error {
return fmt.Errorf("configuration: %s", desc)
}

// A QuorumCallError is used to report that a quorum call failed.
Expand Down
6 changes: 3 additions & 3 deletions mgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ func (m *RawManager) Size() (nodes int) {
func (m *RawManager) AddNode(node *RawNode) error {
if _, found := m.Node(node.ID()); found {
// Node IDs must be unique
return fmt.Errorf("node ID %d already exists (%s)", node.ID(), node.Address())
return configurationError(fmt.Sprintf("node %d (%s) already exists", node.ID(), node.Address()))
}
if m.logger != nil {
m.logger.Printf("connecting to %s with id %d\n", node, node.id)
m.logger.Printf("Connecting to %s with id %d\n", node, node.id)
}
if err := node.connect(m); err != nil {
return fmt.Errorf("connection failed for %s: %w", node, err)
return err
}

m.mu.Lock()
Expand Down
4 changes: 2 additions & 2 deletions mgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ func TestManagerAddNode(t *testing.T) {
id uint32
err string
}{
{"127.0.1.1:1234", 1, "node ID 1 already exists (127.0.1.1:1234)"},
{"127.0.1.1:1234", 1, "configuration: node 1 (127.0.1.1:1234) already exists"},
{"127.0.1.1:1234", 5, ""},
{"127.0.1.1:1234", 6, ""}, // TODO(meling) does it make sense to allow same addr:port for different IDs?
{"127.0.1.1:1234", 2, "node ID 2 already exists (127.0.1.1:1234)"},
{"127.0.1.1:1234", 2, "configuration: node 2 (127.0.1.1:1234) already exists"},
}
for _, test := range tests {
node, err := gorums.NewRawNodeWithID(test.addr, test.id)
Expand Down
4 changes: 2 additions & 2 deletions node.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type RawNode struct {
func NewRawNode(addr string) (*RawNode, error) {
tcpAddr, err := net.ResolveTCPAddr("tcp", addr)
if err != nil {
return nil, fmt.Errorf("node error: '%s' error: %v", addr, err)
return nil, err
}
h := fnv.New32a()
_, _ = h.Write([]byte(tcpAddr.String()))
Expand All @@ -50,7 +50,7 @@ func NewRawNode(addr string) (*RawNode, error) {
func NewRawNodeWithID(addr string, id uint32) (*RawNode, error) {
tcpAddr, err := net.ResolveTCPAddr("tcp", addr)
if err != nil {
return nil, fmt.Errorf("node error: '%s' error: %v", addr, err)
return nil, err
}
return &RawNode{
id: id,
Expand Down

0 comments on commit 907a8e7

Please sign in to comment.