[tor-commits] [snowflake/master] Handle generated errors in client

[cohosh at torproject.org]

commit b26c7a7a7330586c3be3ece02c68999bb279ff40
Author: Shane Howearth <Shane.h.1 at gmail.com>
Date:   Tue Sep 24 09:00:13 2019 +1000

    Handle generated errors in client
---
 client/lib/snowflake.go | 10 +++++-----
 client/lib/webrtc.go    | 11 ++++++++---
 client/snowflake.go     | 21 +++++++++++++++------
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/client/lib/snowflake.go b/client/lib/snowflake.go
index 4af3d88..1c9c34d 100644
--- a/client/lib/snowflake.go
+++ b/client/lib/snowflake.go
@@ -56,18 +56,18 @@ func Handler(socks SocksConnector, snowflakes SnowflakeCollector) error {
 
 // Exchanges bytes between two ReadWriters.
 // (In this case, between a SOCKS and WebRTC connection.)
-func copyLoop(a, b io.ReadWriter) {
+func copyLoop(WebRTC, SOCKS io.ReadWriter) {
 	var wg sync.WaitGroup
 	wg.Add(2)
 	go func() {
-		if _, err := io.Copy(ORPort, WebRTC); err != nil {
-			log.Printf("copying WebRTC to ORPort resulted in error: %v", err)
+		if _, err := io.Copy(SOCKS, WebRTC); err != nil {
+			log.Printf("copying WebRTC to SOCKS resulted in error: %v", err)
 		}
 		wg.Done()
 	}()
 	go func() {
-		if _, err := io.Copy(WebRTC, ORPort); err != nil {
-			log.Printf("copying ORPort to WebRTC resulted in error: %v", err)
+		if _, err := io.Copy(WebRTC, SOCKS); err != nil {
+			log.Printf("copying SOCKS to WebRTC resulted in error: %v", err)
 		}
 		wg.Done()
 	}()
diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go
index 7d361ef..84494d5 100644
--- a/client/lib/webrtc.go
+++ b/client/lib/webrtc.go
@@ -138,7 +138,8 @@ func (c *WebRTCPeer) Connect() error {
 	}
 	err = c.establishDataChannel()
 	if err != nil {
-		return errors.New("WebRTC: Could not establish DataChannel.")
+		// nolint: golint
+		return errors.New("WebRTC: Could not establish DataChannel")
 	}
 	err = c.exchangeSDP()
 	if err != nil {
@@ -151,7 +152,9 @@ func (c *WebRTCPeer) Connect() error {
 // Create and prepare callbacks on a new WebRTC PeerConnection.
 func (c *WebRTCPeer) preparePeerConnection() error {
 	if nil != c.pc {
-		c.pc.Close()
+		if err := c.pc.Close(); err != nil {
+			log.Printf("c.pc.Close returned error: %v", err)
+		}
 		c.pc = nil
 	}
 
@@ -267,7 +270,9 @@ func (c *WebRTCPeer) establishDataChannel() error {
 		if err != nil {
 			// TODO: Maybe shouldn't actually close.
 			log.Println("Error writing to SOCKS pipe")
-			c.writePipe.CloseWithError(err)
+			if inerr := c.writePipe.CloseWithError(err); inerr != nil {
+				log.Printf("c.writePipe.CloseWithError returned error: %v", inerr)
+			}
 		}
 		if n != len(msg.Data) {
 			log.Println("Error: short write")
diff --git a/client/snowflake.go b/client/snowflake.go
index 01c89d8..883a665 100644
--- a/client/snowflake.go
+++ b/client/snowflake.go
@@ -45,7 +45,7 @@ func ConnectLoop(snowflakes sf.SnowflakeCollector) {
 }
 
 // Accept local SOCKS connections and pass them to the handler.
-func socksAcceptLoop(ln *pt.SocksListener, snowflakes sf.SnowflakeCollector) error {
+func socksAcceptLoop(ln *pt.SocksListener, snowflakes sf.SnowflakeCollector) {
 	defer ln.Close()
 	log.Println("Started SOCKS listener.")
 	for {
@@ -55,7 +55,8 @@ func socksAcceptLoop(ln *pt.SocksListener, snowflakes sf.SnowflakeCollector) err
 			if e, ok := err.(net.Error); ok && e.Temporary() {
 				continue
 			}
-			return err
+			log.Printf("SOCKS accept error: %s", err)
+			break
 		}
 		log.Println("SOCKS accepted: ", conn.Req)
 		err = sf.Handler(conn, snowflakes)
@@ -155,7 +156,9 @@ func main() {
 		log.Fatal(err)
 	}
 	if ptInfo.ProxyURL != nil {
-		pt.ProxyError("proxy is not supported")
+		if err := pt.ProxyError("proxy is not supported"); err != nil {
+			log.Printf("call to pt.ProxyError generated error: %v", err)
+		}
 		os.Exit(1)
 	}
 	listeners := make([]net.Listener, 0)
@@ -165,14 +168,18 @@ func main() {
 			// TODO: Be able to recover when SOCKS dies.
 			ln, err := pt.ListenSocks("tcp", "127.0.0.1:0")
 			if err != nil {
-				pt.CmethodError(methodName, err.Error())
+				if inerr := pt.CmethodError(methodName, err.Error()); inerr != nil {
+					log.Printf("handling error generated by pt.ListenSocks with pt.CmethodError generated error: %v", inerr)
+				}
 				break
 			}
 			go socksAcceptLoop(ln, snowflakes)
 			pt.Cmethod(methodName, ln.Version(), ln.Addr())
 			listeners = append(listeners, ln)
 		default:
-			pt.CmethodError(methodName, "no such method")
+			if err := pt.CmethodError(methodName, "no such method"); err != nil {
+				log.Printf("calling pt.CmethodError generated error: %v", err)
+			}
 		}
 	}
 	pt.CmethodsDone()
@@ -186,7 +193,9 @@ func main() {
 		// This environment variable means we should treat EOF on stdin
 		// just like SIGTERM: https://bugs.torproject.org/15435.
 		go func() {
-			io.Copy(ioutil.Discard, os.Stdin)
+			if _, err := io.Copy(ioutil.Discard, os.Stdin); err != nil {
+				log.Printf("calling io.Copy(ioutil.Discard, os.Stdin) returned error: %v", err)
+			}
 			log.Printf("synthesizing SIGTERM because of stdin close")
 			sigChan <- syscall.SIGTERM
 		}()