CAMEL-25671: camel-milo - report a node with an unknown namespace URI instead of failing the connection by etzel-mes · Pull Request #27906 · apache/camel

etzel-mes · 2026-10-15T15:04:11Z

Description

CAMEL-25671, raised in the review of #27780.

A consumer node ID whose namespace URI (nsu=) is not in the namespace table of the server made SubscriptionManager.lookupNamespace throw a NullPointerException. It went to the reconnect path, so one wrong entry in a nodes list took the whole client connection down, including the consumers of the other nodes.

The consumer now treats such a node like a node that does not exist on the server (#27780): one WARN with the node ID, and one DataValue with Bad_NodeIdUnknown for that node (with omitNullValues=false). The other nodes and the connection keep working.

The badSubscriptions map and the 10 second resubscribe that served the old Bad_InvalidArgument branch are removed: that branch was unreachable, because lookupNamespace never returned null.

Test: MonitorNodesTest#unknownNamespaceUriReceivesBadStatusAndTheOtherNodesKeepWorking (fails without the fix: none of the 3 expected messages arrive)
Docs sentence, catalog doc copy, 4.24 upgrade-guide note
Producers (write, call, read, browse) are not changed; they still fail the connection on an unknown nsu=. I can file a separate JIRA for them if wanted.

JIRA user: mardonius (I cannot assign the issue myself).

@davsclaus @oscerd could you take a look?

Target

I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

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

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

🤖 Generated with Claude Code

davsclaus

Thanks for the clear write-up and the focused fix. The NPE is real on main: lookupNamespace(...).get() ends in NodeIds.toNodeId, whose requireNonNull(namespaceIndex) throws for an unknown nsu=, so the consumer took the reconnect path for the whole client. You are also right that the node == null branch (badSubscriptions / resubscribe) could never run, so removing it is safe.

The fix follows the same approach as #27780 (one WARN, one DataValue with a bad status, the other nodes and the connection keep running), and the WARN and PendingValue handling match the existing operation-result branch. The new test covers both halves: the bad status for the unknown namespace, and a later write to node1 still reaching the consumer. The docs, the catalog copy and the 4.24 upgrade-guide note are all in place.

A few non-blocking notes are inline. On the producers (write, call, read, browse): yes, please file the follow-up JIRA you offered.

This review checks the PR against the project's rules and conventions; it does not replace CodeRabbit, Sourcery or SonarCloud, and nothing was built or run locally.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

gnodet-bot

Clean fix. Replacing the hard crash with a graceful null return for unknown namespace URIs is the right approach, the dead badSubscriptions/resubscribe code removal is well-justified, and the test covers the failure mode directly.

Two minor observations:

lookupNamespaceIndex(uri).get() blocks the thread (line 295 of SubscriptionManager.java). This is the same pattern as the original lookupNamespace(s.getNodeId()).get() — no regression introduced here, but worth a follow-up to make this async if the OPC-UA thread model requires it.
Producer path still has the NPE-on-unknown-namespace — the original lookupNamespace method is untouched and producers can still crash the same way. Out of scope for this fix, but worth a follow-up issue.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

etzel-mes · 2026-10-15T15:28:48Z

Thanks both. The producer follow-up is filed as https://issues.apache.org/jira/browse/CAMEL-25690 (write, call, read, browse: the same NPE reaches handleConnectionFailure; write and call also swallow it and complete with null). I will send a PR for it after this one and fold the two lookups into one helper there, as suggested.

Claude Code on behalf of etzel-mes

github-actions · 2026-10-15T15:51:50Z

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 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 have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

github-actions · 2026-10-15T16:58:58Z

🧪 CI tested the following changed modules:

catalog/camel-catalog
components/camel-milo
docs

🔬 Scalpel shadow comparison — Scalpel: 8 of 706 tested, 25 compile-only — current: 8 all tested

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)

Modules Scalpel would test (8)

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/MonitorNodesTest.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

Modules with tests skipped (25)

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

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (35 modules, 5m 41s total)

Total reactor time: 5m 41s

Module	Duration	Status
Camel :: Launcher	51.5s	SUCCESS
Camel :: JBang :: MCP	43.9s	SUCCESS
Camel :: JBang :: Plugin :: Validate	39.2s	SUCCESS
Camel :: Catalog :: Camel Catalog	23.7s	SUCCESS
Camel :: Component DSL	23.2s	SUCCESS
Camel :: JBang :: Plugin :: Kubernetes	21.4s	SUCCESS
Camel :: YAML DSL :: Validator	20.1s	SUCCESS
Camel :: YAML DSL	18.9s	SUCCESS
Camel :: JBang :: Plugin :: Testing	16.7s	SUCCESS
Camel :: Docs	14.8s	SUCCESS
Camel :: Kamelet Main	11.7s	SUCCESS
Camel :: YAML DSL :: Deserializers	8.4s	SUCCESS
Camel :: Catalog :: Camel Route Parser	7.3s	SUCCESS
Camel :: Catalog :: Camel Report Maven Plugin	7.0s	SUCCESS
Camel :: YAML DSL :: Validator Maven Plugin	5.1s	SUCCESS
Camel :: All Components Sync point	5.1s	SUCCESS
Camel :: Catalog :: Maven	3.1s	SUCCESS
Camel :: YAML DSL :: Maven Plugins	2.9s	SUCCESS
Camel :: Catalog :: Suggest (deprecated)	2.7s	SUCCESS
Camel :: Assembly	1.7s	SUCCESS
Camel :: JBang :: Plugin :: Edit	1.7s	SUCCESS
Camel :: JBang :: Plugin :: MCP	1.7s	SUCCESS
Camel :: JBang :: Plugin :: Generate	1.6s	SUCCESS
Camel :: Coverage	1.6s	SUCCESS
Camel :: Catalog :: Dummy Component	1.5s	SUCCESS
Camel :: Endpoint DSL :: Support	1.1s	SUCCESS
Camel :: JBang :: Integration tests	1.0s	SUCCESS
Camel :: JBang :: Main	0.9s	SUCCESS
Camel :: Catalog :: Console	0.8s	SUCCESS
Camel :: Launcher :: Container	0.8s	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 (51.5s)
Camel :: JBang :: MCP (43.9s)
Camel :: JBang :: Plugin :: Validate (39.2s)
Camel :: Catalog :: Camel Catalog (23.7s)
Camel :: Component DSL (23.2s)
Camel :: JBang :: Plugin :: Kubernetes (21.4s)
Camel :: YAML DSL :: Validator (20.1s)
Camel :: YAML DSL (18.9s)
Camel :: JBang :: Plugin :: Testing (16.7s)
Camel :: Docs (14.8s)
Camel :: Kamelet Main (11.7s)
Camel :: YAML DSL :: Deserializers (8.4s)
Camel :: Catalog :: Camel Route Parser (7.3s)
Camel :: Catalog :: Camel Report Maven Plugin (7.0s)
Camel :: YAML DSL :: Validator Maven Plugin (5.1s)
Camel :: All Components Sync point (5.1s)
Camel :: Catalog :: Maven (3.1s)
Camel :: YAML DSL :: Maven Plugins (2.9s)
Camel :: Catalog :: Suggest (deprecated) (2.7s)
Camel :: Assembly (1.7s)

⚙️ View full build and test results

…n when its value is null [CAMEL-25626](https://issues.apache.org/jira/browse/CAMEL-25626). With the default `omitNullValues=true`, the `milo-client` consumer dropped every data change whose value was null, regardless of its status code. OPC UA Part 4 requires a server to send a null value together with a Bad status, so dropping null values drops every Bad status: a sensor failure, a removed node or a monitored item that could not be created (#27780, #27906) never reached the route unless `omitNullValues=false` was set. Why this matters in automation: a route that feeds a historian, an MES, a UNS topic or an alarm evaluates the last value it received. When the quality of a tag goes Bad and the route never hears about it, the stale value keeps being treated as current, and the plant acts on a number that nobody is measuring any more. Quality propagation is the first thing an OT integration is expected to get right; the component should deliver it by default. Closes #27908 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

… instead of failing the connection A consumer node ID whose namespace URI (nsu=) is not in the namespace table of the server made lookupNamespace throw a NullPointerException. The exception went to the reconnect path, so one wrong entry in a nodes list took the whole client connection down, including the consumers of the other nodes. The consumer now treats such a node like a node that does not exist on the server: one WARN with the node ID and one DataValue with Bad_NodeIdUnknown for that node (visible with omitNullValues=false); the other nodes and the connection keep working. The badSubscriptions map and the 10 second resubscribe that served the old Bad_InvalidArgument branch are removed. That branch could not be reached, because lookupNamespace never returned null. Producers (write, call, read, browse) are not changed. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

gnodet-bot

Following up on my earlier review — the fix is solid and the new commit introduces no new issues.

The open thread about lookupNamespaceIndex(uri).get() blocking the thread remains: the author acknowledged it and filed CAMEL-25690 for the follow-up. That is a pre-existing pattern preserved by this PR, not introduced by it — acceptable here.

Previous findings are otherwise all addressed. LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

davsclaus

Thanks @etzel-mes, I re-checked the new revision. The Java change is the commit I approved, rebased onto main. The other differences follow #27908 (CAMEL-25626), which delivers a Bad status whatever omitNullValues says: the test URI drops omitNullValues=false, and the docs, their catalog copy and the 4.24 note no longer ask for it. On this head MiloClientConsumer.handleValueUpdate only omits a null value with a Good status, so the Bad_NodeIdUnknown value reaches the route with the default omitNullValues=true and the test still covers the fix.

The gnodet-bot note on the blocking lookupNamespaceIndex(uri).get() describes the pattern putSubscriptions already had (lookupNamespace(...).get() on main); the producer side and the shared lookup helper are CAMEL-25690.

Nit: the PR description still says the Bad_NodeIdUnknown value comes "with omitNullValues=false", which #27908 made unnecessary.

Merge state: this PR and #27910 conflict in SubscriptionManager: this PR removes handleSubscriptionError and resubscribe right below where #27910 adds recreate(). Keeping recreate() and lookupMonitoredNode and dropping the two removed methods resolves it; @etzel-mes already offered to rebase whichever goes second.

CI: the workflows on cdaba94 are waiting for approval, so nothing has run on this head yet; this approval assumes they go green.

Checked against the project conventions in CLAUDE.md. This is a rules-and-conventions review and does not replace CodeRabbit, Sourcery or SonarCloud.

Claude Code on behalf of @davsclaus.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

davsclaus · 2026-10-17T08:12:19Z

FYI @etzel-mes: the Build and test failure here (ServerCertificateValidationTest.onlyClientsThatCanValidateTheServerCertificateConnect: mock://trustAll Received message count. Expected: <1> but was: <2>) does not come from this PR. It fails on main too (reproduced locally at 55ed20f). CAMEL-25626 (#27908) and CAMEL-25645 (#27824) were merged 26 seconds apart on 2026-10-16, and neither PR's CI ran with the other change. With CAMEL-25626 the trustAll client now also receives the initial DataValue[value=null, statusCode=Bad] of milo-server:myitem1 before Foo, which the new test does not expect. The test needs to accept that first Bad value, for example by checking only the Good values, as the tests updated in CAMEL-25626 do. Once that is fixed on main, a rebase should make this build green.

Claude Code on behalf of Claus Ibsen. This comment was generated by an AI agent and may contain inaccuracies.

gnodet-bot

Following up on my earlier review — the merge commit brings in main with no changes to the PR's own code.

SubscriptionManager.java, MonitorNodesTest.java, the docs, and the upgrade guide are identical to the commit I reviewed yesterday. The merge only picks up upstream changes (including CAMEL-25645 / #27999, which touched ServerCertificateValidationTest in the same module but is unrelated to this fix). No new findings.

Previous findings are all addressed. LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

davsclaus approved these changes Oct 15, 2026

View reviewed changes

Comment thread ...-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java

Comment thread ...-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java

gnodet-bot reviewed Oct 15, 2026

View reviewed changes

Comment thread ...-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java

etzel-mes mentioned this pull request Oct 15, 2026

CAMEL-25626: camel-milo - deliver a data change with a Bad status even when its value is null #27908

Merged

4 of 5 tasks

github-actions Bot added components catalog docs labels Oct 15, 2026

etzel-mes mentioned this pull request Oct 16, 2026

CAMEL-25691: camel-milo - recreate the subscription when it stops publishing #27910

Merged

5 tasks done

etzel-mes force-pushed the fix/CAMEL-25671-milo-unknown-namespace branch from f405d3a to cdaba94 Compare October 16, 2026 17:47

gnodet-bot reviewed Oct 16, 2026

View reviewed changes

davsclaus approved these changes Oct 16, 2026

View reviewed changes

davsclaus mentioned this pull request Oct 17, 2026

CAMEL-25645: camel-milo - ServerCertificateValidationTest writes the value before the consumers start #27999

Merged

2 tasks done

Merge branch 'main' into fix/CAMEL-25671-milo-unknown-namespace

66621e1

gnodet-bot reviewed Oct 17, 2026

View reviewed changes

davsclaus added this to the 4.24.0 milestone Oct 17, 2026

davsclaus assigned etzel-mes Oct 17, 2026

davsclaus added the bug Something isn't working label Oct 17, 2026

davsclaus merged commit 99af303 into apache:main Oct 17, 2026
7 checks passed

etzel-mes mentioned this pull request Oct 18, 2026

CAMEL-25690: camel-milo - answer a producer node with an unknown namespace URI instead of reconnecting #28045

Merged

4 of 5 tasks

etzel-mes deleted the fix/CAMEL-25671-milo-unknown-namespace branch October 18, 2026 08:31

Navigation Menu

Repository navigation

CAMEL-25671: camel-milo - report a node with an unknown namespace URI instead of failing the connection - #27906

CAMEL-25671: camel-milo - report a node with an unknown namespace URI instead of failing the connection#27906
davsclaus merged 2 commits into
apache:mainfrom
etzel-mes:fix/CAMEL-25671-milo-unknown-namespace

etzel-mes commented Oct 15, 2026

Uh oh!

davsclaus left a comment

Uh oh!

Uh oh!

Uh oh!

gnodet-bot left a comment

Uh oh!

Uh oh!

etzel-mes commented Oct 15, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 15, 2026

Uh oh!

github-actions Bot commented Oct 15, 2026 •

edited

Loading

Uh oh!

gnodet-bot left a comment

Uh oh!

davsclaus left a comment

Uh oh!

davsclaus commented Oct 17, 2026

Uh oh!

gnodet-bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

etzel-mes commented Oct 15, 2026

Description

Target

Tracking

Apache Camel coding standards and style

AI-assisted contributions

Uh oh!

davsclaus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

etzel-mes commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 15, 2026

Uh oh!

github-actions Bot commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

davsclaus left a comment

Choose a reason for hiding this comment

Uh oh!

davsclaus commented Oct 17, 2026

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

etzel-mes commented Oct 15, 2026 •

edited

Loading

github-actions Bot commented Oct 15, 2026 •

edited

Loading