Merge pull request #618 from gopcua/issue-616-log-fatal

client.go

@@ -133,6 +133,15 @@ type Client struct {
 
 	// monitorOnce ensures only one connection monitor is running
 	monitorOnce sync.Once
+
+	// cfgerr contains an error that was captured in ApplyConfig.
+	// Since the API does not allow to bubble the error up in NewClient
+	// and we don't want to break existing code right away we carry the
+	// error here and bubble it up during Dial and Connect.
+	//
+	// Note: Starting with v0.5 NewClient will return the error and this
+	// variable needs to be removed.
+	cfgerr error
 }
 
 // NewClient creates a new Client.
@@ -146,6 +155,8 @@ type Client struct {
 // #Option for details.
 //
 // https://godoc.org/github.com/gopcua/opcua#Option
+//
+// Note: Starting with v0.5 this function will will return an error.
 func NewClient(endpoint string, opts ...Option) *Client {
 	cfg := ApplyConfig(opts...)
 	c := Client{
@@ -156,6 +167,7 @@ func NewClient(endpoint string, opts ...Option) *Client {
 		pendingAcks: make([]*ua.SubscriptionAcknowledgement, 0),
 		pausech:     make(chan struct{}, 2),
 		resumech:    make(chan struct{}, 2),
+		cfgerr:      cfg.Error(), // todo(fs): remove with v0.5.0 and return the error
 	}
 	c.pauseSubscriptions(context.Background())
 	c.setPublishTimeout(uasc.MaxTimeout)
@@ -182,6 +194,11 @@ const (
 
 // Connect establishes a secure channel and creates a new session.
 func (c *Client) Connect(ctx context.Context) (err error) {
+	// todo(fs): remove with v0.5.0
+	if c.cfgerr != nil {
+		return c.cfgerr
+	}
+
 	if c.SecureChannel() != nil {
 		return errors.Errorf("already connected")
 	}
@@ -513,6 +530,11 @@ func (c *Client) monitor(ctx context.Context) {
 
 // Dial establishes a secure channel.
 func (c *Client) Dial(ctx context.Context) error {
+	// todo(fs): remove with v0.5.0
+	if c.cfgerr != nil {
+		return c.cfgerr
+	}
+
 	stats.Client().Add("Dial", 1)
 
 	if c.SecureChannel() != nil {

config.go

@@ -56,6 +56,18 @@ type Config struct {
 	dialer  *uacp.Dialer
 	sechan  *uasc.Config
 	session *uasc.SessionConfig
+	err     error
+}
+
+func (cfg *Config) setError(err error) {
+	if cfg.err != nil {
+		return
+	}
+	cfg.err = err
+}
+
+func (cfg *Config) Error() error {
+	return cfg.err
 }
 
 // NewDialer creates a uacp.Dialer from the config options
@@ -68,6 +80,8 @@ func NewDialer(cfg *Config) *uacp.Dialer {
 
 // ApplyConfig applies the config options to the default configuration.
 // todo(fs): Can we find a better name?
+//
+// Note: Starting with v0.5 this function will will return an error.
 func ApplyConfig(opts ...Option) *Config {
 	cfg := &Config{
 		sechan:  DefaultClientConfig(),
@@ -162,7 +176,8 @@ func RemoteCertificateFile(filename string) Option {
 	return func(cfg *Config) {
 		cert, err := loadCertificate(filename)
 		if err != nil {
-			log.Fatal(err)
+			cfg.setError(err)
+			return
 		}
 		cfg.sechan.RemoteCertificate = cert
 	}
@@ -220,7 +235,8 @@ func PrivateKeyFile(filename string) Option {
 		}
 		key, err := loadPrivateKey(filename)
 		if err != nil {
-			log.Fatal(err)
+			cfg.setError(err)
+			return
 		}
 		cfg.sechan.LocalKey = key
 	}
@@ -267,7 +283,8 @@ func CertificateFile(filename string) Option {
 
 		cert, err := loadCertificate(filename)
 		if err != nil {
-			log.Fatal(err)
+			cfg.setError(err)
+			return
 		}
 		setCertificate(cert, cfg)
 	}

config_test.go

@@ -4,19 +4,21 @@ import (
 	"crypto/rsa"
 	"crypto/tls"
 	"encoding/pem"
+	"fmt"
 	"io/ioutil"
 	"net"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 	"time"
 
+	"github.com/pascaldekloe/goe/verify"
+
 	"github.com/gopcua/opcua/ua"
 	"github.com/gopcua/opcua/uacp"
 	"github.com/gopcua/opcua/uapolicy"
 	"github.com/gopcua/opcua/uasc"
-
-	"github.com/pascaldekloe/goe/verify"
 )
 
 // test certificate generated with
@@ -141,6 +143,16 @@ func TestOptions(t *testing.T) {
 		keyPEMFile  = filepath.Join(d, "key.pem")
 	)
 
+	// the error message for "file not found" is platform dependent.
+	notFoundError := func(msg, name string) error {
+		switch runtime.GOOS {
+		case "windows":
+			return fmt.Errorf("opcua: Failed to load %s: open %s: The system cannot find the file specified.", msg, name)
+		default:
+			return fmt.Errorf("opcua: Failed to load %s: open %s: no such file or directory", msg, name)
+		}
+	}
+
 	if err := ioutil.WriteFile(certDERFile, certDER, 0644); err != nil {
 		t.Fatal(err)
 	}
@@ -283,6 +295,13 @@ func TestOptions(t *testing.T) {
 				}(),
 			},
 		},
+		{
+			name: `CertificateFile() error`,
+			opt:  CertificateFile("x"),
+			cfg: &Config{
+				err: notFoundError("certificate", "x"),
+			},
+		},
 		{
 			name: `Lifetime(10ms)`,
 			opt:  Lifetime(10 * time.Millisecond),
@@ -338,6 +357,13 @@ func TestOptions(t *testing.T) {
 				}(),
 			},
 		},
+		{
+			name: `PrivateKeyFile() error`,
+			opt:  PrivateKeyFile("x"),
+			cfg: &Config{
+				err: notFoundError("private key", "x"),
+			},
+		},
 		{
 			name: `ProductURI("a")`,
 			opt:  ProductURI("a"),
@@ -404,6 +430,13 @@ func TestOptions(t *testing.T) {
 				}(),
 			},
 		},
+		{
+			name: `RemoteCertificateFile() error`,
+			opt:  RemoteCertificateFile("x"),
+			cfg: &Config{
+				err: notFoundError("certificate", "x"),
+			},
+		},
 		{
 			name: `RequestTimeout(5s)`,
 			opt:  RequestTimeout(5 * time.Second),
@@ -764,8 +797,24 @@ func TestOptions(t *testing.T) {
 				tt.cfg.session = DefaultSessionConfig()
 			}
 
+			errstr := func(err error) string {
+				if err != nil {
+					return err.Error()
+				}
+				return ""
+			}
+
 			cfg := ApplyConfig(tt.opt)
-			verify.Values(t, "", cfg, tt.cfg)
+			if got, want := errstr(cfg.Error()), errstr(tt.cfg.err); got != "" || want != "" {
+				if got != want {
+					t.Fatalf("got error %q want %q", got, want)
+				}
+				return
+			}
+			if !verify.Values(t, "", cfg, tt.cfg) {
+				t.Logf("got %#v", cfg)
+				t.Logf("want %#v", tt.cfg)
+			}
 		})
 	}
 }