From 33fe50a8d5650d95d46800187d0b27477eb30717 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:41:43 +0200 Subject: [PATCH 1/9] Fix typo. Provide ctx to the function call instead of context.Background() --- node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node.go b/node.go index 87931a68..c025e697 100644 --- a/node.go +++ b/node.go @@ -31,7 +31,7 @@ func (n *Node) String() string { // Note: Starting with v0.5 this method will require a context // and the corresponding XXXWithContext(ctx) method will be removed. func (n *Node) NodeClass(ctx context.Context) (ua.NodeClass, error) { - return n.NodeClassWithContext(context.Background()) + return n.NodeClassWithContext(ctx) } // Note: Starting with v0.5 this method is superseded by the non 'WithContext' method. From 6836af7dc98443403949550b4656be6f83ef9b16 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:42:45 +0200 Subject: [PATCH 2/9] Add StatusBadSessionIDInvalid to the switch to improve debug messaging On this code we definitely should pause publishing until new session will be created --- client_sub.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client_sub.go b/client_sub.go index 90f878e2..6778f76c 100644 --- a/client_sub.go +++ b/client_sub.go @@ -408,6 +408,10 @@ func (c *Client) publish(ctx context.Context) error { dlog.Printf("error: session not active. pausing publish loop") return err + case err == ua.StatusBadSessionIDInvalid: + dlog.Printf("error: session not valid. pausing publish loop") + return err + case err == ua.StatusBadServerNotConnected: dlog.Printf("error: no connection. pausing publish loop") return err From 864d46913de9e6430b8e87e18f750dbe2a247ca8 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:44:53 +0200 Subject: [PATCH 3/9] Remove return error not needed All err in this function are local to statements --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 29fac722..fcb67c34 100644 --- a/client.go +++ b/client.go @@ -193,7 +193,7 @@ const ( ) // Connect establishes a secure channel and creates a new session. -func (c *Client) Connect(ctx context.Context) (err error) { +func (c *Client) Connect(ctx context.Context) error { // todo(fs): remove with v0.5.0 if c.cfgerr != nil { return c.cfgerr From 3a675c06e8c73ac8eb8ce99bdb9a6647b108a9b6 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:45:21 +0200 Subject: [PATCH 4/9] Disconnect was already set above --- client.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/client.go b/client.go index fcb67c34..b7b79efe 100644 --- a/client.go +++ b/client.go @@ -321,8 +321,6 @@ func (c *Client) monitor(ctx context.Context) { action = createSecureChannel } - c.setState(Disconnected) - c.pauseSubscriptions(ctx) var ( From 7cc1c9c669d545166541189089117e629072ed8b Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:45:40 +0200 Subject: [PATCH 5/9] Set reconnecting status when recreating session --- client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client.go b/client.go index b7b79efe..58f5b53c 100644 --- a/client.go +++ b/client.go @@ -385,6 +385,8 @@ func (c *Client) monitor(ctx context.Context) { // This only works if the session is still open on the server // otherwise recreate it + c.setState(Reconnecting) + s := c.Session() if s == nil { dlog.Printf("no session to restore") @@ -414,6 +416,7 @@ func (c *Client) monitor(ctx context.Context) { case recreateSession: dlog.Printf("action: recreateSession") + c.setState(Reconnecting) // create a new session to replace the previous one dlog.Printf("trying to recreate session") From c263e7c81fcfe8602823d0222d662177ccec4237 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:46:43 +0200 Subject: [PATCH 6/9] Interpret StatusBadServiceUnsupported to make clear debug logging This is happens on Siemens PLCs when function on the server is not supported --- client.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/client.go b/client.go index 58f5b53c..3f3fe759 100644 --- a/client.go +++ b/client.go @@ -461,6 +461,12 @@ func (c *Client) monitor(ctx context.Context) { // recreate them all if that fails. res, err := c.transferSubscriptions(ctx, subIDs) switch { + + case errors.Is(err, ua.StatusBadServiceUnsupported): + dlog.Printf("transfer subscriptions not supported. Recreating all subscriptions: %v", err) + subsToRepublish = nil + subsToRecreate = subIDs + case err != nil: dlog.Printf("transfer subscriptions failed. Recreating all subscriptions: %v", err) subsToRepublish = nil From f4e2dafb43624a400a7d38dcf4b7d44650580638 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:54:41 +0200 Subject: [PATCH 7/9] Expose RevisedSessionTimeout on Session. Clients might benefit from knowing the value communicated by OPCUA server --- client.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/client.go b/client.go index 3f3fe759..b52b24ff 100644 --- a/client.go +++ b/client.go @@ -697,6 +697,15 @@ type Session struct { // Session response. Used to generate the signatures for the ActivateSessionRequest // and User Authorization serverNonce []byte + + // revisedTimeout is the actual maximum time that a Session shall remain open without activity. + revisedTimeout time.Duration +} + +// RevisedTimeout return actual maximum time that a Session shall remain open without activity. +// This value is provided by the server in response to CreateSession. +func (s *Session) RevisedTimeout() time.Duration { + return s.revisedTimeout } // CreateSession creates a new session which is not yet activated and not @@ -771,6 +780,7 @@ func (c *Client) CreateSessionWithContext(ctx context.Context, cfg *uasc.Session resp: res, serverNonce: res.ServerNonce, serverCertificate: res.ServerCertificate, + revisedTimeout: time.Duration(res.RevisedSessionTimeout) * time.Millisecond, } return nil From 264533b27007eed3c42c52521da2edd4b826c4b1 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 14:59:17 +0200 Subject: [PATCH 8/9] Fill in endpoint to allow the client to connect --- examples/subscribe/subscribe.go | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/subscribe/subscribe.go b/examples/subscribe/subscribe.go index fb39339c..f3c42689 100644 --- a/examples/subscribe/subscribe.go +++ b/examples/subscribe/subscribe.go @@ -47,6 +47,7 @@ func main() { if ep == nil { log.Fatal("Failed to find suitable endpoint") } + ep.EndpointURL = *endpoint fmt.Println("*", ep.SecurityPolicyURI, ep.SecurityMode) From 1d817a1094d2b94aec8e50f026defa7878ae9807 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 22 May 2023 15:17:20 +0200 Subject: [PATCH 9/9] Add example how to retry common errors --- examples/read/read.go | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/examples/read/read.go b/examples/read/read.go index 27716c91..c1955567 100644 --- a/examples/read/read.go +++ b/examples/read/read.go @@ -6,8 +6,11 @@ package main import ( "context" + "errors" "flag" + "io" "log" + "time" "github.com/gopcua/opcua" "github.com/gopcua/opcua/debug" @@ -44,12 +47,44 @@ func main() { TimestampsToReturn: ua.TimestampsToReturnBoth, } - resp, err := c.ReadWithContext(ctx, req) - if err != nil { - log.Fatalf("Read failed: %s", err) + var resp *ua.ReadResponse + for { + resp, err = c.ReadWithContext(ctx, req) + if err == nil { + break + } + + // Following switch contains known errors that can be retried by the user. + // Best practice is to do it on read operations. + switch { + case err == io.EOF && c.State() != opcua.Closed: + // has to be retried unless user closed the connection + time.After(1 * time.Second) + continue + + case errors.Is(err, ua.StatusBadSessionIDInvalid): + // Session is not activated has to be retried. Session will be recreated internally. + time.After(1 * time.Second) + continue + + case errors.Is(err, ua.StatusBadSessionNotActivated): + // Session is invalid has to be retried. Session will be recreated internally. + time.After(1 * time.Second) + continue + + case errors.Is(err, ua.StatusBadSecureChannelIDInvalid): + // secure channel will be recreated internally. + time.After(1 * time.Second) + continue + + default: + log.Fatalf("Read failed: %s", err) + } } - if resp.Results[0].Status != ua.StatusOK { + + if resp != nil && resp.Results[0].Status != ua.StatusOK { log.Fatalf("Status not OK: %v", resp.Results[0].Status) } + log.Printf("%#v", resp.Results[0].Value.Value()) }