Skip to content

Conversation

@erossignon
Copy link
Contributor

@erossignon erossignon commented Oct 27, 2025

code clean up

remove redundant findBasicType Code( as already provided by node-opcua with better implementation)

fix issue: Connection Sharing Bug in OPC UA Client

Root Cause

A recent change in how the href field is constructed introduced a bug in connection sharing. Previously, the href and opcua:nodeid were separate fields, like this:

{
  "href": "opc.tcp://localhost:4840",
  "opcua:nodeid": "ns=1;s=Temperature"
}

Now, the href includes the node ID as a query parameter:

{
  "href": "opc.tcp://localhost:4840/?id=ns=1;s=Temperature"
}

note: both syntaxes are still supported

Because the href is now unique for each node, the OPCAProtocolClient treats each request as requiring a new connection. This leads to session exhaustion on the server, as connections are not being reused.

Why this happens

The href was previously used as a key to identify and reuse existing connections. With the new scheme, every hrefis unique, so the client creates a new connection for each operation.

the fix
To restore connection sharing, the href is first normalized by removing the query parameters before using it as a connection key. This ensures that connections to the same endpoint (e.g., opc.tcp://localhost:4840) are reused, regardless of the node ID in the query.

In the meantime, a refactor and simplification of the connection sharing mecanism has been undertaken

Refactor codec:

A commit refactors the codec logic in the OPCUA binding into multiple files.
The main codec.ts file has been split into:

  • opcua-binary-codec.ts
  • opcua-data-schemas.ts
  • `opcua-json-codec.ts

@erossignon erossignon force-pushed the opcua-binding-refactor branch from f12f153 to 1b26f48 Compare October 27, 2025 11:28
@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 89.00862% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.65%. Comparing base (3325e12) to head (1b26f48).
⚠️ Report is 36 commits behind head on master.

Files with missing lines Patch % Lines
...kages/binding-opcua/src/codecs/opcua-json-codec.ts 76.77% 34 Missing and 2 partials ⚠️
...ackages/binding-opcua/src/opcua-protocol-client.ts 88.80% 14 Missing ⚠️
...ges/binding-opcua/src/codecs/opcua-binary-codec.ts 98.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1451      +/-   ##
==========================================
+ Coverage   77.58%   77.65%   +0.07%     
==========================================
  Files          79       86       +7     
  Lines       15331    15903     +572     
  Branches     1445     1501      +56     
==========================================
+ Hits        11894    12349     +455     
- Misses       3414     3531     +117     
  Partials       23       23              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@danielpeintner danielpeintner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Minor: copyright year could/should be updated

Copy link
Member

@danielpeintner danielpeintner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the latest change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants