You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "etzel-mes (via GitHub)" <gi...@apache.org> on 2026/10/18 07:36:57 UTC

[PR] CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting [camel]

etzel-mes opened a new pull request, #28045:
URL: https://github.com/apache/camel/pull/28045

   # Description
   
   [CAMEL-25690](https://issues.apache.org/jira/browse/CAMEL-25690).
   
   The `milo-client` producer (write, method call, read) and the `milo-browse` producer resolve the namespace URI (`nsu=`) of a node ID through the namespace table of the server. When the URI was not in the table, the lookup failed with a `NullPointerException`, and the producer took it for a lost connection: the client reconnected, which also recreated the subscriptions of the consumers that share the connection, and a read lost the values of its other nodes.
   
   Why this matters in automation: a namespace URI is configuration, and it goes stale when a server is reconfigured or replaced. A route with a stale URI then reconnected the client on every request. I checked this against a server that is not built on Milo (Microsoft opc-plc 2.15.4, on the OPC Foundation .NET stack). Without this change, one awaited write to such a node replaced the `OpcUaClient` of the connection, and the requests sent right after it failed with `No connected`: a write and two method calls that the server would have answered itself. One such browse reconnected the browse connection.
   
   The change:
   - The consumer and the producers now share one lookup, as suggested in the review of CAMEL-25671 (#27906). `lookupNamespace` resolves the namespace URI or index, completes with `null` for a URI that the server does not know, and replaces the consumer's `lookupMonitoredNode`. The consumer reports such a node with `Bad_NodeIdUnknown` and resolves it under the lock, both as before; its lookup still blocks while the monitored items are created, as before.
   - A producer answers such a node the way a server answers a node that it does not have (OPC 10000-4 v1.05, Table 179, and Table 61 for the Call service), without sending it to the server: a write returns `Bad_NodeIdUnknown`; a read returns `Bad_NodeIdUnknown` for that node in its position, and a browse under that node ID, both with the results of the other nodes; a method call returns `Bad_NodeIdUnknown` for an unknown object and `Bad_MethodInvalid` for an unknown method. A request without such a node takes the same path as before. A read or browse of only such nodes sends no request, because a server answers an empty one with `Bad_NothingToDo`, which the producer takes for a lost connection. A DEBUG line names the node, because a write that does not wait for its result reports nothing else.
   - New `ProducerUnknownNamespaceTest`: a write, and reads and browses of such a node next to a known node and alone, get these results, and a method call with an unknown method or object completes without an error. Each connection keeps its `OpcUaClient`, and after the write, the reads and the calls a consumer on the same connection receives the next value. Without the fix all four tests fail, and the client reconnects in each.
   - Component docs (Node ID section, with its catalog copy) and a 4.24 upgrade-guide note.
   
   Against opc-plc with the change, each answer matched the answer of the server for a node it does not have (`ns=99;s=…`, or an unknown identifier in a known namespace): one read of six nodes returned `Good, Good, Bad_NodeIdUnknown, Good, Bad_NodeIdUnknown, Bad_NodeIdUnknown` in the requested order, and a browse returned the references of `Objects` next to `Bad_NodeIdUnknown`. Both connections kept their `OpcUaClient`, and a consumer on the client connection kept receiving its counter. On top of #27956, the two method calls also returned what the server returns for an unknown method of an object (`Bad_MethodInvalid`) and for an unknown object (`Bad_NodeIdUnknown`).
   
   This PR does not change the four request guards in `SubscriptionManager` that #27956 (CAMEL-25694, @gnodet) changes, and the two merge without conflicts in either order. The call guard on main drops every method call result, so for now a method call with such a node only keeps the connection, and its answer reaches the exchange with #27956. The docs and the test therefore leave the call answer out; I can add it to both once #27956 is merged. On top of #27956, such call assertions and the tests of #27956 already pass.
   
   The unchecked lookup dates back to the first client implementation and came back with the component in 84b7e90419c; this change does not revert any later work.
   
   Checked with `-Psourcecheck clean verify` on the module (79 tests), four more runs of the new and the affected test classes with `-Dsurefire.rerunFailingTestsCount=0`, one of them with `-XX:ActiveProcessorCount=1`, and the new test together with the tests of #27956 on top of it.
   
   @davsclaus @oscerd could you take a look?
   
   # Target
   
   - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch)
   
   # Tracking
   - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it).
   
   # Apache Camel coding standards and style
   
   - [x] I checked that each commit in the pull request has a meaningful subject line and body.
   
   - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes.
   
   # AI-assisted contributions
   
   - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used.
   
   _Claude Code on behalf of etzel-mes_
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28045:
URL: https://github.com/apache/camel/pull/28045#issuecomment-6251193659

   <!-- ci-tested-modules -->
   :test_tube: **CI tested the following changed modules:**
   
   - `catalog/camel-catalog`
   - `components/camel-milo`
   - `docs`
   
   ---
   
   <details><summary>:microscope: Scalpel shadow comparison — Scalpel: 8 of 706 tested, 25 compile-only — current: 8 all tested</summary>
   
   [Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **8 affected modules** (current approach: 8).
   
   **Skip-tests mode would test 8 modules** (3 direct + 7 downstream), **skip tests for 25** (generated code, meta-modules)
   
   <details><summary>Modules Scalpel would test (8)</summary>
   
   - `camel-jbang-mcp` ← downstream of org.apache.camel:camel-catalog
   - `camel-jbang-plugin-mcp` ← downstream of org.apache.camel:camel-jbang-core
   - `camel-jbang-plugin-tui` ← downstream of org.apache.camel:camel-catalog
   - `camel-jbang-plugin-validate` ← downstream of org.apache.camel:camel-yaml-dsl-validator
   - `camel-launcher-container` ← downstream of org.apache.camel:camel-launcher
   - `camel-milo` ← components/camel-milo/src/main/docs/milo-client-component.adoc, components/camel-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java, components/camel-milo/src/test/java/org/apache/camel/component/milo/ProducerUnknownNamespaceTest.java
   - `camel-yaml-dsl-validator` ← downstream of org.apache.camel:camel-catalog
   - `camel-yaml-dsl-validator-maven-plugin` ← downstream of org.apache.camel:camel-yaml-dsl-validator
   
   </details>
   
   <details><summary>Modules with tests skipped (25)</summary>
   
   - `apache-camel`
   - `camel-allcomponents`
   - `camel-catalog-console`
   - `camel-catalog-maven`
   - `camel-catalog-suggest`
   - `camel-componentdsl`
   - `camel-endpointdsl`
   - `camel-endpointdsl-support`
   - `camel-itest`
   - `camel-jbang-core`
   - `camel-jbang-it`
   - `camel-jbang-main`
   - `camel-jbang-plugin-edit`
   - `camel-jbang-plugin-generate`
   - `camel-jbang-plugin-kubernetes`
   - `camel-jbang-plugin-test`
   - `camel-kamelet-main`
   - `camel-launcher`
   - `camel-report-maven-plugin`
   - `camel-route-parser`
   - `camel-yaml-dsl`
   - `camel-yaml-dsl-deserializers`
   - `camel-yaml-dsl-maven-plugin`
   - `coverage`
   - `dummy-component`
   
   </details>
   
   > :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)
   
   </details>
   
   <details><summary>All tested modules (35 modules, 4m 46s total)</summary>
   
   **Total reactor time:** 4m 46s
   
   | Module | Duration | Status |
   | --- | --- | --- |
   | Camel :: Launcher | 41.6s | SUCCESS |
   | Camel :: JBang :: MCP | 34.3s | SUCCESS |
   | Camel :: JBang :: Plugin :: Validate | 27.7s | SUCCESS |
   | Camel :: Catalog :: Camel Catalog | 20.9s | SUCCESS |
   | Camel :: YAML DSL :: Validator | 19.9s | SUCCESS |
   | Camel :: Component DSL | 18.9s | SUCCESS |
   | Camel :: YAML DSL | 18.4s | SUCCESS |
   | Camel :: Docs | 14.0s | SUCCESS |
   | Camel :: Catalog :: Camel Report Maven Plugin | 12.1s | SUCCESS |
   | Camel :: JBang :: Plugin :: Kubernetes | 11.2s | SUCCESS |
   | Camel :: Kamelet Main | 10.8s | SUCCESS |
   | Camel :: JBang :: Plugin :: Testing | 8.9s | SUCCESS |
   | Camel :: YAML DSL :: Deserializers | 7.2s | SUCCESS |
   | Camel :: Catalog :: Camel Route Parser | 7.1s | SUCCESS |
   | Camel :: YAML DSL :: Validator Maven Plugin | 6.3s | SUCCESS |
   | Camel :: Catalog :: Suggest (deprecated) | 5.8s | SUCCESS |
   | Camel :: All Components Sync point | 4.1s | SUCCESS |
   | Camel :: Catalog :: Maven | 2.5s | SUCCESS |
   | Camel :: YAML DSL :: Maven Plugins | 2.4s | SUCCESS |
   | Camel :: JBang :: Integration tests | 1.5s | SUCCESS |
   | Camel :: JBang :: Plugin :: Edit | 1.4s | SUCCESS |
   | Camel :: Catalog :: Dummy Component | 1.3s | SUCCESS |
   | Camel :: Assembly | 1.3s | SUCCESS |
   | Camel :: Coverage | 1.1s | SUCCESS |
   | Camel :: Endpoint DSL :: Support | 1.1s | SUCCESS |
   | Camel :: Catalog :: Console | 1.0s | SUCCESS |
   | Camel :: JBang :: Plugin :: Generate | 0.9s | SUCCESS |
   | Camel :: JBang :: Main | 0.8s | SUCCESS |
   | Camel :: Launcher :: Container | 0.7s | SUCCESS |
   | Camel :: JBang :: Plugin :: MCP | 0.6s | SUCCESS |
   | Camel :: Endpoint DSL | n/a |  |
   | Camel :: Integration Tests | n/a |  |
   | Camel :: JBang :: Core | n/a |  |
   | Camel :: JBang :: Plugin :: TUI | n/a |  |
   | Camel :: Milo | n/a |  |
   
   **Top 20 slowest modules:**
   - `Camel :: Launcher` (41.6s)
   - `Camel :: JBang :: MCP` (34.3s)
   - `Camel :: JBang :: Plugin :: Validate` (27.7s)
   - `Camel :: Catalog :: Camel Catalog` (20.9s)
   - `Camel :: YAML DSL :: Validator` (19.9s)
   - `Camel :: Component DSL` (18.9s)
   - `Camel :: YAML DSL` (18.4s)
   - `Camel :: Docs` (14.0s)
   - `Camel :: Catalog :: Camel Report Maven Plugin` (12.1s)
   - `Camel :: JBang :: Plugin :: Kubernetes` (11.2s)
   - `Camel :: Kamelet Main` (10.8s)
   - `Camel :: JBang :: Plugin :: Testing` (8.9s)
   - `Camel :: YAML DSL :: Deserializers` (7.2s)
   - `Camel :: Catalog :: Camel Route Parser` (7.1s)
   - `Camel :: YAML DSL :: Validator Maven Plugin` (6.3s)
   - `Camel :: Catalog :: Suggest (deprecated)` (5.8s)
   - `Camel :: All Components Sync point` (4.1s)
   - `Camel :: Catalog :: Maven` (2.5s)
   - `Camel :: YAML DSL :: Maven Plugins` (2.4s)
   - `Camel :: JBang :: Integration tests` (1.5s)
   
   </details>
   
   ---
   :gear: [View full build and test results](https://github.com/apache/camel/actions/runs/39110444833)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28045:
URL: https://github.com/apache/camel/pull/28045#issuecomment-6250972250

   :star2: Thank you for your contribution to the Apache Camel project! :star2:
     :robot: CI automation will test this PR automatically.
   
     :camel: Apache Camel Committers, please review the following items:
   
     * First-time contributors **require MANUAL approval** for the GitHub Actions to run
     * You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot although they are normally detected and executed by CI.
     * You can label PRs using `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
     * Build and test logs are available in the summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary.
     
     :warning: Be careful when sharing logs. Review their contents before sharing them publicly.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting [camel]

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #28045:
URL: https://github.com/apache/camel/pull/28045#discussion_r4318288502


##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java:
##########
@@ -445,14 +437,24 @@ public void dispose() {
             }
         }
 
+        /**
+         * @return the node ID with the namespace index of the server, completed with null if its namespace URI is not
+         *         in the namespace table of the server: such a node does not exist on the server
+         */
         private CompletableFuture<NodeId> lookupNamespace(final ExpandedNodeId nodeId) {
             LOG.trace("Expanded Node Id: {}", nodeId);
 
             final String uri = nodeId.getNamespaceUri();
 
             if (uri != null) {
                 LOG.trace("Looking up namespace: {}", uri);
-                return lookupNamespaceIndex(uri).thenApply(index -> toNodeId(index, nodeId));
+                return lookupNamespaceIndex(uri).thenApply(index -> {
+                    if (index == null) {

Review Comment:
   Resolving: this nit is optional and does not block the merge. A follow-up can make `lookupNamespaceIndex` fail on a bad status if it turns up in practice.
   
   _Claude Code on behalf of Claus Ibsen_



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting [camel]

Posted by "etzel-mes (via GitHub)" <gi...@apache.org>.
etzel-mes commented on PR #28045:
URL: https://github.com/apache/camel/pull/28045#issuecomment-6247489250

   Rebased onto main after #27910 was merged. The conflicts were only textual. The `lookupMonitoredNode` that this PR replaces with the shared lookup sat next to the new `recreate`; `recreate` calls `putSubscriptions`, so it now uses the shared lookup too. The two camel-milo notes in the 4.24 upgrade guide were next to each other. Otherwise the change is the same: `-Psourcecheck clean verify` on the module passes with 83 tests, and the branch still merges with #27956 without conflicts.
   
   _Claude Code on behalf of etzel-mes_
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting [camel]

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus merged PR #28045:
URL: https://github.com/apache/camel/pull/28045


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting [camel]

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #28045:
URL: https://github.com/apache/camel/pull/28045#discussion_r4307398183


##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java:
##########
@@ -445,14 +437,24 @@ public void dispose() {
             }
         }
 
+        /**
+         * @return the node ID with the namespace index of the server, completed with null if its namespace URI is not
+         *         in the namespace table of the server: such a node does not exist on the server
+         */
         private CompletableFuture<NodeId> lookupNamespace(final ExpandedNodeId nodeId) {
             LOG.trace("Expanded Node Id: {}", nodeId);
 
             final String uri = nodeId.getNamespaceUri();
 
             if (uri != null) {
                 LOG.trace("Looking up namespace: {}", uri);
-                return lookupNamespaceIndex(uri).thenApply(index -> toNodeId(index, nodeId));
+                return lookupNamespaceIndex(uri).thenApply(index -> {
+                    if (index == null) {

Review Comment:
   Nit: `lookupNamespaceIndex` also completes with `null` when the read of `Server_NamespaceArray` returns a value that is not a `String[]`, for example a bad status for that read. That case now looks like an unknown URI: `Bad_NodeIdUnknown`, and this DEBUG line. It is rare, the result is not cached so the next request tries again, and the consumer has behaved the same way since CAMEL-25671. So it is fine to leave as it is. You could make `lookupNamespaceIndex` fail the future on a bad status, so that a failed lookup is not reported as an unknown namespace.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org