Skip to content

Commit

Permalink
Remove error state from ClientConn (#27)
Browse files Browse the repository at this point in the history
Currently method errors are stored in ClientConn.lastError and requires callers
of ClientConn.readResponse to check this field after each call. This is error
prone as it hides the fact that readResponse can fail.

This change removes the ClientConn.lastError field and instead make
ClientConn.readResponse return an error along with the segment. Calling code is
updated accordingly.
  • Loading branch information
Bjørn authored Mar 8, 2021
1 parent c2adff6 commit e7da5bb
Showing 1 changed file with 12 additions and 18 deletions.
30 changes: 12 additions & 18 deletions internal/vici/clientConn.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package vici

import (
"fmt"
"io"
"net"
"time"
)
Expand All @@ -17,7 +16,6 @@ type ClientConn struct {
conn net.Conn
responseChan chan segment
eventHandlers map[string]func(response map[string]interface{})
lastError error

// ReadTimeout specifies a time limit for requests made
// by this client.
Expand All @@ -26,7 +24,6 @@ type ClientConn struct {

func (c *ClientConn) Close() error {
close(c.responseChan)
c.lastError = io.ErrClosedPipe
return c.conn.Close()
}

Expand Down Expand Up @@ -79,25 +76,22 @@ func (c *ClientConn) Request(apiname string, concretePayload interface{}) (map[s
return nil, fmt.Errorf("writing segment: %w", err)
}

outMsg := c.readResponse()
if c.lastError != nil {
return nil, fmt.Errorf("read response: %w", c.lastError)
outMsg, err := c.readResponse()
if err != nil {
return nil, fmt.Errorf("read response: %w", err)
}
if outMsg.typ != stCMD_RESPONSE {
return nil, fmt.Errorf("[%s] response error %d", apiname, outMsg.typ)
}
return outMsg.msg, nil
}

func (c *ClientConn) readResponse() segment {
func (c *ClientConn) readResponse() (segment, error) {
select {
case outMsg := <-c.responseChan:
return outMsg
return outMsg, nil
case <-time.After(c.ReadTimeout):
if c.lastError == nil {
c.lastError = fmt.Errorf("timeout waiting for message response")
}
return segment{}
return segment{}, fmt.Errorf("timeout waiting for message response")
}
}

Expand All @@ -114,10 +108,10 @@ func (c *ClientConn) RegisterEvent(name string, handler func(response map[string
delete(c.eventHandlers, name)
return fmt.Errorf("write segment: %w", err)
}
outMsg := c.readResponse()
if c.lastError != nil {
outMsg, err := c.readResponse()
if err != nil {
delete(c.eventHandlers, name)
return fmt.Errorf("read response: %w", c.lastError)
return fmt.Errorf("read response: %w", err)
}

if outMsg.typ != stEVENT_CONFIRM {
Expand All @@ -135,9 +129,9 @@ func (c *ClientConn) UnregisterEvent(name string) error {
if err != nil {
return fmt.Errorf("write segment: %w", err)
}
outMsg := c.readResponse()
if c.lastError != nil {
return fmt.Errorf("read response: %w", c.lastError)
outMsg, err := c.readResponse()
if err != nil {
return fmt.Errorf("read response: %w", err)
}

if outMsg.typ != stEVENT_CONFIRM {
Expand Down

0 comments on commit e7da5bb

Please sign in to comment.