Convert conn.Err into logging.

Also, remove all error-side-effect testing cos it was a bit shit.
First step on the long road to refactoring the nick/chan state tracking
and making everything more testable and mockable with interfaces.

client/connection.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"crypto/tls"
 	"github.com/fluffle/goirc/event"
+	"github.com/fluffle/goirc/logging"
 	"fmt"
 	"net"
 	"os"
@@ -16,7 +17,6 @@ const (
 )
 
 // An IRC connection is represented by this struct.
-// Once connected, any errors encountered are piped down *Conn.Err.
 type Conn struct {
 	// Connection Hostname and Nickname
 	Host    string
@@ -46,9 +46,6 @@ type Conn struct {
 	// Control channels to goroutines
 	cSend, cLoop chan bool
 
-	// Error channel to transmit any fail back to the user
-	Err chan os.Error
-
 	// Misc knobs to tweak client behaviour:
 	// Are we connecting via SSL? Do we care about certificate validity?
 	SSL       bool
@@ -62,13 +59,6 @@ type Conn struct {
 
 	// Internal counters for flood protection
 	badness, lastsent int64
-
-	// Function which returns a *time.Time for use as a timestamp
-	Timestamp func() *time.Time
-
-	// Enable debugging? Set format for timestamps on debug output.
-	Debug    bool
-	TSFormat string
 }
 
 // Creates a new IRC connection object, but doesn't connect to anything so
@@ -80,7 +70,6 @@ func New(nick, user, name string) *Conn {
 		Dispatcher: reg,
 		in:         make(chan *Line, 32),
 		out:        make(chan string, 32),
-		Err:        make(chan os.Error, 4),
 		cSend:      make(chan bool),
 		cLoop:      make(chan bool),
 		SSL:        false,
@@ -89,8 +78,6 @@ func New(nick, user, name string) *Conn {
 		Flood:      false,
 		badness:    0,
 		lastsent:   0,
-		Timestamp:  time.LocalTime,
-		TSFormat:   "15:04:05",
 	}
 	conn.initialise()
 	conn.SetupHandlers()
@@ -128,6 +115,7 @@ func (conn *Conn) Connect(host string, pass ...string) os.Error {
 		if !hasPort(host) {
 			host += ":6697"
 		}
+		logging.Info("irc.Connect(): Connecting to %s with SSL.", host)
 		if s, err := tls.Dial("tcp", host, conn.SSLConfig); err == nil {
 			conn.sock = s
 		} else {
@@ -137,6 +125,7 @@ func (conn *Conn) Connect(host string, pass ...string) os.Error {
 		if !hasPort(host) {
 			host += ":6667"
 		}
+		logging.Info("irc.Connect(): Connecting to %s without SSL.", host)
 		if s, err := net.Dial("tcp", host); err == nil {
 			conn.sock = s
 		} else {
@@ -166,11 +155,6 @@ func (conn *Conn) postConnect() {
 	go conn.runLoop()
 }
 
-// dispatch a nicely formatted os.Error to the error channel
-func (conn *Conn) error(s string, a ...interface{}) {
-	conn.Err <- os.NewError(fmt.Sprintf(s, a...))
-}
-
 // copied from http.client for great justice
 func hasPort(s string) bool {
 	return strings.LastIndex(s, ":") > strings.LastIndex(s, "]")
@@ -194,18 +178,15 @@ func (conn *Conn) recv() {
 	for {
 		s, err := conn.io.ReadString('\n')
 		if err != nil {
-			conn.error("irc.recv(): %s", err.String())
+			logging.Error("irc.recv(): %s", err.String())
 			conn.shutdown()
-			break
+			return
 		}
 		s = strings.Trim(s, "\r\n")
-		t := conn.Timestamp()
+		logging.Debug("<- %s", s)
-		if conn.Debug {
-			fmt.Println(t.Format(conn.TSFormat) + " <- " + s)
-		}
 
 		if line := parseLine(s); line != nil {
-			line.Time = t
+			line.Time = time.LocalTime()
 			conn.in <- line
 		}
 	}
@@ -232,14 +213,12 @@ func (conn *Conn) write(line string) {
 	}
 
 	if _, err := conn.io.WriteString(line + "\r\n"); err != nil {
-		conn.error("irc.send(): %s", err.String())
+		logging.Error("irc.send(): %s", err.String())
 		conn.shutdown()
 		return
 	}
 	conn.io.Flush()
-	if conn.Debug {
+	logging.Debug("-> %s", line)
-		fmt.Println(conn.Timestamp().Format(conn.TSFormat) + " -> " + line)
-	}
 }
 
 // Implement Hybrid's flood control algorithm to rate-limit outgoing lines.
@@ -257,6 +236,8 @@ func (conn *Conn) rateLimit(chars int64) {
 	// calculation above, then we're at risk of "Excess Flood".
 	if conn.badness > 10*second && !conn.Flood {
 		// so sleep for the current line's time value before sending it
+		logging.Debug("irc.rateLimit(): Flood! Sleeping for %.2f secs.",
+			float64(linetime)/float64(second))
 		time.Sleep(linetime)
 	}
 }
@@ -265,6 +246,7 @@ func (conn *Conn) shutdown() {
 	// Guard against double-call of shutdown() if we get an error in send()
 	// as calling sock.Close() will cause recv() to recieve EOF in readstring()
 	if conn.Connected {
+		logging.Info("irc.shutdown(): Disconnected from server.")
 		conn.Dispatcher.Dispatch("disconnected", conn, &Line{})
 		conn.Connected = false
 		conn.sock.Close()

client/connection_test.go

@@ -1,11 +1,18 @@
 package client
 
 import (
+	"github.com/fluffle/goirc/logging"
 	"strings"
 	"testing"
 	"time"
 )
 
+func init() {
+	// We have Error level logging that is printed on socket shutdown
+	// which we don't care too much about when testing with a mock conn...
+	logging.SetLogLevel(logging.LogFatal)
+}
+
 func setUp(t *testing.T) (*mockNetConn, *Conn) {
 	c := New("test", "test", "Testing IRC")
 	c.State = t
@@ -39,33 +46,7 @@ func setUp(t *testing.T) (*mockNetConn, *Conn) {
 }
 
 func tearDown(m *mockNetConn, c *Conn) {
-	// This is enough to cause all the associated goroutines in m and c stop
+	c.shutdown()
-	// (tested below in TestShutdown to make sure this is the case)
-	m.Close()
-}
-
-func (conn *Conn) ExpectError() {
-	// With the current error handling, we could block on reading the Err
-	// channel, so ensure we don't wait forever with a 5ms timeout.
-	t := conn.State.(*testing.T)
-	timer := time.NewTimer(5e6)
-	select {
-	case <-timer.C:
-		t.Errorf("Error expected on Err channel, none received.")
-	case <-conn.Err:
-		timer.Stop()
-	}
-}
-
-func (conn *Conn) ExpectNoErrors() {
-	t := conn.State.(*testing.T)
-	timer := time.NewTimer(5e6)
-	select {
-	case <-timer.C:
-	case err := <-conn.Err:
-		timer.Stop()
-		t.Errorf("No error expected on Err channel, received:\n\t%s", err)
-	}
 }
 
 func TestShutdown(t *testing.T) {
@@ -78,18 +59,6 @@ func TestShutdown(t *testing.T) {
 	// Call shutdown manually
 	c.shutdown()
 
-	// Check that we get an EOF from Read()
-	timer := time.NewTimer(5e6)
-	select {
-	case <-timer.C:
-		t.Errorf("No error received for shutdown.")
-	case err := <-c.Err:
-		timer.Stop()
-		if err.String() != "irc.recv(): EOF" {
-			t.Errorf("Expected EOF, got: %s", err)
-		}
-	}
-
 	// Verify that the connection no longer thinks it's connected
 	if c.Connected {
 		t.Errorf("Conn still thinks it's connected to the server.")
@@ -116,17 +85,9 @@ func TestEOF(t *testing.T) {
 	// Simulate EOF from server
 	m.Close()
 
-	// Check that we get an EOF from Read()
+	// Since things happen in different internal goroutines, we need to wait
-	timer := time.NewTimer(5e6)
+	// 1 ms should be enough :-)
-	select {
+	time.Sleep(1e6)
-	case <-timer.C:
-		t.Errorf("No error received for shutdown.")
-	case err := <-c.Err:
-		timer.Stop()
-		if err.String() != "irc.recv(): EOF" {
-			t.Errorf("Expected EOF, got: %s", err)
-		}
-	}
 
 	// Verify that the connection no longer thinks it's connected
 	if c.Connected {

client/handlers.go

@@ -5,6 +5,7 @@ package client
 
 import (
 	"github.com/fluffle/goirc/event"
+	"github.com/fluffle/goirc/logging"
 	"strings"
 )
 
@@ -75,7 +76,7 @@ func (conn *Conn) h_NICK(line *Line) {
 	if n := conn.GetNick(line.Nick); n != nil {
 		n.ReNick(line.Args[0])
 	} else {
-		conn.error("irc.NICK(): buh? unknown nick %s.", line.Nick)
+		logging.Warn("irc.NICK(): unknown nick %s.", line.Nick)
 	}
 }
 
@@ -96,9 +97,8 @@ func (conn *Conn) h_JOIN(line *Line) {
 		// first we've seen of this channel, so should be us joining it
 		// NOTE this will also take care of n == nil && ch == nil
 		if n != conn.Me {
-			conn.error("irc.JOIN(): buh? JOIN to unknown channel %s recieved"+
+			logging.Warn("irc.JOIN(): JOIN to unknown channel %s recieved " +
-				"from (non-me) nick %s",
+				"from (non-me) nick %s", line.Args[0], line.Nick)
-				line.Args[0], line.Nick)
 			return
 		}
 		ch = conn.NewChannel(line.Args[0])
@@ -128,11 +128,11 @@ func (conn *Conn) h_PART(line *Line) {
 		if _, ok := ch.Nicks[n]; ok {
 			ch.DelNick(n)
 		} else {
-			conn.error("irc.PART(): nick %s is not on channel %s",
+			logging.Warn("irc.PART(): nick %s is not on channel %s",
 				line.Nick, line.Args[0])
 		}
 	} else {
-		conn.error("irc.PART(): buh? PART of channel %s by nick %s",
+		logging.Warn("irc.PART(): PART of channel %s by nick %s",
 			line.Args[0], line.Nick)
 	}
 }
@@ -147,11 +147,11 @@ func (conn *Conn) h_KICK(line *Line) {
 		if _, ok := ch.Nicks[n]; ok {
 			ch.DelNick(n)
 		} else {
-			conn.error("irc.KICK(): nick %s is not on channel %s",
+			logging.Warn("irc.KICK(): nick %s is not on channel %s",
 				line.Nick, line.Args[0])
 		}
 	} else {
-		conn.error("irc.KICK(): buh? KICK from channel %s of nick %s",
+		logging.Warn("irc.KICK(): KICK from channel %s of nick %s",
 			line.Args[0], line.Args[1])
 	}
 }
@@ -161,7 +161,7 @@ func (conn *Conn) h_QUIT(line *Line) {
 	if n := conn.GetNick(line.Nick); n != nil {
 		n.Delete()
 	} else {
-		conn.error("irc.QUIT(): buh? QUIT from unknown nick %s", line.Nick)
+		logging.Warn("irc.QUIT(): QUIT from unknown nick %s", line.Nick)
 	}
 }
 
@@ -173,12 +173,14 @@ func (conn *Conn) h_MODE(line *Line) {
 	} else if n := conn.GetNick(line.Args[0]); n != nil {
 		// nick mode change, should be us
 		if n != conn.Me {
-			conn.error("irc.MODE(): buh? recieved MODE %s for (non-me) nick %s", line.Args[0], n.Nick)
+			logging.Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s",
+				line.Args[0], n.Nick)
 			return
 		}
 		conn.ParseNickModes(n, line.Args[1])
 	} else {
-		conn.error("irc.MODE(): buh? not sure what to do with MODE %s", strings.Join(line.Args, " "))
+		logging.Warn("irc.MODE(): not sure what to do with MODE %s",
+			strings.Join(line.Args, " "))
 	}
 }
 
@@ -187,7 +189,8 @@ func (conn *Conn) h_TOPIC(line *Line) {
 	if ch := conn.GetChannel(line.Args[0]); ch != nil {
 		ch.Topic = line.Args[1]
 	} else {
-		conn.error("irc.TOPIC(): buh? topic change on unknown channel %s", line.Args[0])
+		logging.Warn("irc.TOPIC(): topic change on unknown channel %s",
+			line.Args[0])
 	}
 }
 
@@ -198,7 +201,8 @@ func (conn *Conn) h_311(line *Line) {
 		n.Host = line.Args[3]
 		n.Name = line.Args[5]
 	} else {
-		conn.error("irc.311(): buh? received WHOIS info for unknown nick %s", line.Args[1])
+		logging.Warn("irc.311(): received WHOIS info for unknown nick %s",
+			line.Args[1])
 	}
 }
 
@@ -207,7 +211,8 @@ func (conn *Conn) h_324(line *Line) {
 	if ch := conn.GetChannel(line.Args[1]); ch != nil {
 		conn.ParseChannelModes(ch, line.Args[2], line.Args[3:])
 	} else {
-		conn.error("irc.324(): buh? received MODE settings for unknown channel %s", line.Args[1])
+		logging.Warn("irc.324(): received MODE settings for unknown channel %s",
+			line.Args[1])
 	}
 }
 
@@ -216,7 +221,8 @@ func (conn *Conn) h_332(line *Line) {
 	if ch := conn.GetChannel(line.Args[1]); ch != nil {
 		ch.Topic = line.Args[2]
 	} else {
-		conn.error("irc.332(): buh? received TOPIC value for unknown channel %s", line.Args[1])
+		logging.Warn("irc.332(): received TOPIC value for unknown channel %s",
+			line.Args[1])
 	}
 }
 
@@ -237,7 +243,8 @@ func (conn *Conn) h_352(line *Line) {
 			n.Modes.Invisible = true
 		}
 	} else {
-		conn.error("irc.352(): buh? got WHO reply for unknown nick %s", line.Args[5])
+		logging.Warn("irc.352(): received WHO reply for unknown nick %s",
+			line.Args[5])
 	}
 }
 
@@ -280,7 +287,8 @@ func (conn *Conn) h_353(line *Line) {
 			}
 		}
 	} else {
-		conn.error("irc.353(): buh? received NAMES list for unknown channel %s", line.Args[2])
+		logging.Warn("irc.353(): received NAMES list for unknown channel %s",
+			line.Args[2])
 	}
 }
 
@@ -289,7 +297,8 @@ func (conn *Conn) h_671(line *Line) {
 	if n := conn.GetNick(line.Args[1]); n != nil {
 		n.Modes.SSL = true
 	} else {
-		conn.error("irc.671(): buh? received WHOIS SSL info for unknown nick %s", line.Args[1])
+		logging.Warn("irc.671(): received WHOIS SSL info for unknown nick %s",
+			line.Args[1])
 	}
 }
 

client/handlers_test.go

@@ -73,8 +73,7 @@ func TestNICK(t *testing.T) {
 
 	// Call handler with a NICK line changing "our" nick to test1.
 	c.h_NICK(parseLine(":test!test@somehost.com NICK :test1"))
-	// Should generate no errors and no response to server
+	// Should generate no response to server
-	c.ExpectNoErrors()
 	m.ExpectNothing()
 
 	// Verify that our Nick has changed
@@ -87,7 +86,6 @@ func TestNICK(t *testing.T) {
 
 	// Call handler with a NICK line changing user1 to somebody
 	c.h_NICK(parseLine(":user1!ident1@host1.com NICK :somebody"))
-	c.ExpectNoErrors()
 	m.ExpectNothing()
 
 	if c.GetNick("user1") != nil {
@@ -99,7 +97,6 @@ func TestNICK(t *testing.T) {
 
 	// Send a NICK line for an unknown nick.
 	c.h_NICK(parseLine(":blah!moo@cows.com NICK :milk"))
-	c.ExpectError()
 	m.ExpectNothing()
 }
 
@@ -134,10 +131,8 @@ func TestJOIN(t *testing.T) {
 	// verifying their expected side-effects instead. Fixing this requires
 	// significant effort to move Conn to being a mockable interface type
 	// instead of a concrete struct. I'm not sure how feasible this is :-/
-	//
+	// 
-	// Instead, in this test we (so far) just verify the correct code paths
+	// Soon, we'll find out :-)
-	// are followed and trust that the unit tests for the various methods
-	// ensure that they do the right thing.
 
 	m, c := setUp(t)
 	defer tearDown(m, c)
@@ -146,9 +141,6 @@ func TestJOIN(t *testing.T) {
 	// Call handler with JOIN by test to #test1
 	c.h_JOIN(parseLine(":test!test@somehost.com JOIN :#test1"))
 
-	// Ensure we aren't triggering an error here
-	c.ExpectNoErrors()
-
 	// Verify that the MODE and WHO commands are sent correctly
 	m.Expect("MODE #test1")
 	m.Expect("WHO #test1")
@@ -162,9 +154,6 @@ func TestJOIN(t *testing.T) {
 	// OK, now #test1 exists, JOIN another user we don't know about
 	c.h_JOIN(parseLine(":user1!ident1@host1.com JOIN :#test1"))
 
-	// Again, expect no errors
-	c.ExpectNoErrors()
-
 	// Verify that the WHO command is sent correctly
 	m.Expect("WHO user1")
 
@@ -179,7 +168,6 @@ func TestJOIN(t *testing.T) {
 	c.h_JOIN(parseLine(":user2!ident2@host2.com JOIN :#test1"))
 
 	// We already know about this user and channel, so nothing should be sent
-	c.ExpectNoErrors()
 	m.ExpectNothing()
 
 	// Simple verification that the state tracking has actually been done
@@ -189,12 +177,10 @@ func TestJOIN(t *testing.T) {
 
 	// Test error paths -- unknown channel, unknown nick
 	c.h_JOIN(parseLine(":blah!moo@cows.com JOIN :#test2"))
-	c.ExpectError()
 	m.ExpectNothing()
 
 	// unknown channel, known nick that isn't Me.
 	c.h_JOIN(parseLine(":user2!ident2@host2.com JOIN :#test2"))
-	c.ExpectError()
 	m.ExpectNothing()
 }
 
@@ -217,8 +203,7 @@ func TestPART(t *testing.T) {
 	// Then make them PART
 	c.h_PART(parseLine(":user1!ident1@host1.com PART #test1 :Bye!"))
 
-	// Expect no errors or output
+	// Expect no output
-	c.ExpectNoErrors()
 	m.ExpectNothing()
 
 	// Quick check of tracking code
@@ -229,19 +214,15 @@ func TestPART(t *testing.T) {
 	// Test error states.
 	// Part a known user from a known channel they are not on.
 	c.h_PART(parseLine(":user1!ident1@host1.com PART #test1 :Bye!"))
-	c.ExpectError()
 
 	// Part an unknown user from a known channel.
 	c.h_PART(parseLine(":user2!ident2@host2.com PART #test1 :Bye!"))
-	c.ExpectError()
 
 	// Part a known user from an unknown channel.
 	c.h_PART(parseLine(":user1!ident1@host1.com PART #test3 :Bye!"))
-	c.ExpectError()
 
 	// Part an unknown user from an unknown channel.
 	c.h_PART(parseLine(":user2!ident2@host2.com PART #test3 :Bye!"))
-	c.ExpectError()
 }
 
 // Test the handler for KICK messages
@@ -264,8 +245,7 @@ func TestKICK(t *testing.T) {
 	// Then kick them!
 	c.h_KICK(parseLine(":test!test@somehost.com KICK #test1 user1 :Bye!"))
 
-	// Expect no errors or output
+	// Expect no output
-	c.ExpectNoErrors()
 	m.ExpectNothing()
 
 	// Quick check of tracking code
@@ -276,19 +256,15 @@ func TestKICK(t *testing.T) {
 	// Test error states.
 	// Kick a known user from a known channel they are not on.
 	c.h_KICK(parseLine(":test!test@somehost.com KICK #test1 user1 :Bye!"))
-	c.ExpectError()
 
 	// Kick an unknown user from a known channel.
 	c.h_KICK(parseLine(":test!test@somehost.com KICK #test2 user2 :Bye!"))
-	c.ExpectError()
 
 	// Kick a known user from an unknown channel.
 	c.h_KICK(parseLine(":test!test@somehost.com KICK #test3 user1 :Bye!"))
-	c.ExpectError()
 
 	// Kick an unknown user from an unknown channel.
 	c.h_KICK(parseLine(":test!test@somehost.com KICK #test4 user2 :Bye!"))
-	c.ExpectError()
 }
 
 // Test the handler for QUIT messages
@@ -310,8 +286,7 @@ func TestQUIT(t *testing.T) {
 	// Have user1 QUIT
 	c.h_QUIT(parseLine(":user1!ident1@host1.com QUIT :Bye!"))
 
-	// Expect no errors or output
+	// Expect no output
-	c.ExpectNoErrors()
 	m.ExpectNothing()
 
 	// Quick check of tracking code
@@ -326,11 +301,9 @@ func TestQUIT(t *testing.T) {
 
 	// Have user1 QUIT again, expect ERRORS!
 	c.h_QUIT(parseLine(":user1!ident1@host1.com QUIT :Bye!"))
-	c.ExpectError()
 
 	// Have a previously unmentioned user quit, expect an error
 	c.h_QUIT(parseLine(":user2!ident2@host2.com QUIT :Bye!"))
-	c.ExpectError()
 }
 
 // Test the handler for MODE messages
@@ -354,9 +327,8 @@ func TestMODE(t *testing.T) {
 	// Send a channel mode line
 	c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test1 +kisvo somekey test user1"))
 
-	// Shouldn't get any errors or output
+	// Expect no output
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Verify expected state afterwards.
 	if cp := user1.Channels[test1]; !(cp.Op || c.Me.Channels[test1].Voice ||
@@ -373,7 +345,6 @@ func TestMODE(t *testing.T) {
 	// Send a nick mode line
 	c.h_MODE(parseLine(":test!test@somehost.com MODE test +ix"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Verify the two modes we expect to change did so
 	if !nm.Invisible || nm.WallOps || !nm.HiddenHost {
@@ -383,12 +354,10 @@ func TestMODE(t *testing.T) {
 	// Check error paths -- send a valid user mode that's not us
 	c.h_MODE(parseLine(":user1!ident1@host1.com MODE user1 +w"))
 	m.ExpectNothing()
-	c.ExpectError()
 
 	// Send a random mode for an unknown channel
 	c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test2 +is"))
 	m.ExpectNothing()
-	c.ExpectError()
 }
 
 // Test the handler for TOPIC messages
@@ -407,7 +376,6 @@ func TestTOPIC(t *testing.T) {
 	// Send a TOPIC line
 	c.h_TOPIC(parseLine(":user1!ident1@host1.com TOPIC #test1 :something something"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Make sure the channel's topic has been changed
 	if test1.Topic != "something something" {
@@ -417,7 +385,6 @@ func TestTOPIC(t *testing.T) {
 	// Check error paths -- send a topic for an unknown channel
 	c.h_TOPIC(parseLine(":user1!ident1@host1.com TOPIC #test2 :dark side"))
 	m.ExpectNothing()
-	c.ExpectError()
 }
 
 // Test the handler for 311 / RPL_WHOISUSER
@@ -431,7 +398,6 @@ func Test311(t *testing.T) {
 	// Send a 311 reply
 	c.h_311(parseLine(":irc.server.org 311 test user1 ident1 host1.com * :name"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Verify we now know more about user1
 	if user1.Ident != "ident1" ||
@@ -443,7 +409,6 @@ func Test311(t *testing.T) {
 	// Check error paths -- send a 311 for an unknown nick
 	c.h_311(parseLine(":irc.server.org 311 test user2 ident2 host2.com * :dongs"))
 	m.ExpectNothing()
-	c.ExpectError()
 }
 
 // Test the handler for 324 / RPL_CHANNELMODEIS
@@ -463,7 +428,6 @@ func Test324(t *testing.T) {
 	// Send a 324 reply
 	c.h_324(parseLine(":irc.server.org 324 test #test1 +snk somekey"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Make sure the modes we expected to be set were set and vice versa
 	if !cm.Secret || !cm.NoExternalMsg || cm.Moderated || cm.Key != "somekey" {
@@ -473,7 +437,6 @@ func Test324(t *testing.T) {
 	// Check unknown channel causes an error
 	c.h_324(parseLine(":irc.server.org 324 test #test2 +pmt"))
 	m.ExpectNothing()
-	c.ExpectError()
 }
 
 // Test the handler for 332 / RPL_TOPIC
@@ -492,7 +455,6 @@ func Test332(t *testing.T) {
 	// Send a 332 reply
 	c.h_332(parseLine(":irc.server.org 332 test #test1 :something something"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Make sure the channel's topic has been changed
 	if test1.Topic != "something something" {
@@ -502,7 +464,6 @@ func Test332(t *testing.T) {
 	// Check unknown channel causes an error
 	c.h_324(parseLine(":irc.server.org 332 test #test2 :dark side"))
 	m.ExpectNothing()
-	c.ExpectError()
 }
 
 // Test the handler for 352 / RPL_WHOREPLY
@@ -516,7 +477,6 @@ func Test352(t *testing.T) {
 	// Send a 352 reply
 	c.h_352(parseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 G :0 name"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Verify we now know more about user1
 	if user1.Ident != "ident1" ||
@@ -530,7 +490,6 @@ func Test352(t *testing.T) {
 	// Check that modes are set correctly from WHOREPLY
 	c.h_352(parseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 H* :0 name"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	if !user1.Modes.Invisible || !user1.Modes.Oper {
 		t.Errorf("WHO modes of user1 not set correctly.")
@@ -539,7 +498,6 @@ func Test352(t *testing.T) {
 	// Check error paths -- send a 352 for an unknown nick
 	c.h_352(parseLine(":irc.server.org 352 test #test2 ident2 host2.com irc.server.org user2 G :0 fooo"))
 	m.ExpectNothing()
-	c.ExpectError()
 }
 
 // Test the handler for 353 / RPL_NAMREPLY
@@ -575,7 +533,6 @@ func Test353(t *testing.T) {
 	c.h_353(parseLine(":irc.server.org 353 test = #test1 :test @user1 user2 +voice "))
 	c.h_353(parseLine(":irc.server.org 353 test = #test1 :%halfop @op &admin ~owner "))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	if len(test1.Nicks) != 8 {
 		t.Errorf("Unexpected number of nicks in test channel after 353.")
@@ -616,7 +573,6 @@ func Test353(t *testing.T) {
 	// Check unknown channel causes an error
 	c.h_324(parseLine(":irc.server.org 353 test = #test2 :test ~user3"))
 	m.ExpectNothing()
-	c.ExpectError()
 }
 
 // Test the handler for 671 (unreal specific)
@@ -633,7 +589,6 @@ func Test671(t *testing.T) {
 	// Send a 671 reply
 	c.h_671(parseLine(":irc.server.org 671 test user1 :some ignored text"))
 	m.ExpectNothing()
-	c.ExpectNoErrors()
 
 	// Ensure user1 is now known to be on an SSL connection
 	if !user1.Modes.SSL {
@@ -643,5 +598,4 @@ func Test671(t *testing.T) {
 	// Check error paths -- send a 671 for an unknown nick
 	c.h_671(parseLine(":irc.server.org 671 test user2 :some ignored text"))
 	m.ExpectNothing()
-	c.ExpectError()
 }

client/line.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"github.com/fluffle/goirc/logging"
 	"strings"
 	"time"
 )
@@ -33,6 +34,7 @@ func parseLine(s string) *Line {
 			line.Src, s = s[1:idx], s[idx+1:]
 		} else {
 			// pretty sure we shouldn't get here ...
+			logging.Warn("Parsing of line '%s' didn't go well.", s)
 			line.Src = s[1:]
 			return line
 		}

client/nickchan.go

@@ -5,6 +5,7 @@ package client
 
 import (
 	"fmt"
+	"github.com/fluffle/goirc/logging"
 	"reflect"
 	"strconv"
 )
@@ -129,14 +130,16 @@ func (conn *Conn) ParseChannelModes(ch *Channel, modes string, modeargs []string
 			if len(modeargs) != 0 {
 				ch.Modes.Key, modeargs = modeargs[0], modeargs[1:]
 			} else {
-				conn.error("irc.ParseChanModes(): buh? not enough arguments to process MODE %s %s%s", ch.Name, modestr, m)
+				logging.Warn("irc.ParseChanModes(): not enough arguments to " +
+					"process MODE %s %s%s", ch.Name, modestr, m)
 			}
 		case 'l':
 			if len(modeargs) != 0 {
 				ch.Modes.Limit, _ = strconv.Atoi(modeargs[0])
 				modeargs = modeargs[1:]
 			} else {
-				conn.error("irc.ParseChanModes(): buh? not enough arguments to process MODE %s %s%s", ch.Name, modestr, m)
+				logging.Warn("irc.ParseChanModes(): not enough arguments to " +
+					"process MODE %s %s%s", ch.Name, modestr, m)
 			}
 		case 'q', 'a', 'o', 'h', 'v':
 			if len(modeargs) != 0 {
@@ -156,10 +159,12 @@ func (conn *Conn) ParseChannelModes(ch *Channel, modes string, modeargs []string
 					}
 					modeargs = modeargs[1:]
 				} else {
-					conn.error("irc.ParseChanModes(): MODE %s %s%s %s: buh? state tracking failure.", ch.Name, modestr, m, modeargs[0])
+					logging.Warn("irc.ParseChanModes(): untracked nick %s " +
+						"recieved MODE on channel %s", modeargs[0], ch.Name)
 				}
 			} else {
-				conn.error("irc.ParseChanModes(): buh? not enough arguments to process MODE %s %s%s", ch.Name, modestr, m)
+				logging.Warn("irc.ParseChanModes(): not enough arguments to " +
+					"process MODE %s %s%s", ch.Name, modestr, m)
 			}
 		}
 	}
@@ -203,7 +208,8 @@ func (ch *Channel) AddNick(n *Nick) {
 		ch.Nicks[n] = new(ChanPrivs)
 		n.Channels[ch] = ch.Nicks[n]
 	} else {
-		ch.conn.error("irc.Channel.AddNick() warning: trying to add already-present nick %s to channel %s", n.Nick, ch.Name)
+		logging.Warn("irc.Channel.AddNick(): trying to add already-present " +
+			"nick %s to channel %s", n.Nick, ch.Name)
 	}
 }
 
@@ -251,7 +257,8 @@ func (n *Nick) AddChannel(ch *Channel) {
 		ch.Nicks[n] = new(ChanPrivs)
 		n.Channels[ch] = ch.Nicks[n]
 	} else {
-		n.conn.error("irc.Nick.AddChannel() warning: trying to add already-present channel %s to nick %s", ch.Name, n.Nick)
+		logging.Warn("irc.Nick.AddChannel(): trying to add already-present " +
+			"channel %s to nick %s", ch.Name, n.Nick)
 	}
 }
 
@@ -401,7 +408,7 @@ func (n *Nick) String() string {
 //	+npk key
 func (cm *ChanMode) String() string {
 	str := "+"
-	a := make([]string, 2)
+	a := make([]string, 0)
 	v := reflect.Indirect(reflect.ValueOf(cm))
 	t := v.Type()
 	for i := 0; i < v.NumField(); i++ {
@@ -413,12 +420,12 @@ func (cm *ChanMode) String() string {
 		case reflect.String:
 			if f.String() != "" {
 				str += ChanModeToString[t.Field(i).Name]
-				a[0] = f.String()
+				a = append(a, f.String())
 			}
 		case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
 			if f.Int() != 0 {
 				str += ChanModeToString[t.Field(i).Name]
-				a[1] = fmt.Sprintf("%d", f.Int())
+				a = append(a, fmt.Sprintf("%d", f.Int()))
 			}
 		}
 	}