fix: don't return continuation points on BadNoContinuationPoints - #4022
Merged
marcschier merged 4 commits intoJul 20, 2026
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4022 +/- ##
==========================================
- Coverage 73.85% 73.53% -0.33%
==========================================
Files 1345 1345
Lines 179988 180039 +51
Branches 31668 31678 +10
==========================================
- Hits 132938 132390 -548
- Misses 36299 36943 +644
+ Partials 10751 10706 -45
🚀 New features to boost your workflow:
|
marcschier
approved these changes
Jul 18, 2026
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes OPC UA Browse/BrowseNext behavior to avoid returning usable continuation points when the server reports BadNoContinuationPoints, and adds deterministic regression tests to lock in the correct semantics.
Changes:
- Prevent
Browse/BrowseNextfrom copying/returning continuation points when the operation result is notGood. - Dispose and clear continuation points when allocation fails with
BadNoContinuationPoints. - Add regression tests covering
BrowseandBrowseNextfor continuation-point quota exhaustion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs | Adds deterministic tests and helpers to simulate continuation-point persistence and quota exhaustion. |
| src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs | Ensures continuation points are only returned on success and are disposed/cleared on BadNoContinuationPoints. |
Collaborator
|
@MrAlaskan thanks for all the great contributions, we really appreciate it!!! Could you take a look at the server test which seem to be failing in this PR? |
Contributor
Author
|
Thank you for recognizing my work. I’ll take a look at the failing server tests and start working on resolving them. |
marcschier
pushed a commit
that referenced
this pull request
Aug 18, 2026
…) [backport to master378] (#4268) ### Summary Backport of commit 399747f (#4022) from `master` to `master378`. Fixes `Browse` and `BrowseNext` so they do not return a usable continuation point when the server reports `BadNoContinuationPoints`, and adds regression tests covering both code paths. ### Problem Per OPC UA Part 4, if the server cannot allocate another continuation point for a `Browse`/`BrowseNext` operation, the result should be `Bad_NoContinuationPoints` and the server must not also return a usable continuation point. Previously `FetchReferences` could return `BadNoContinuationPoints` while still passing a live continuation point back to its callers, producing a contradictory response (`BadNoContinuationPoints` with a non-empty `ContinuationPoint`), and `BrowseNext` could overwrite the error with `Good`. ### Changes - Dispose and clear the continuation point when `FetchReferences` hits the no-continuation-points path. - Only copy a continuation point into `Browse` and `BrowseNext` results when the operation completed successfully (`ServiceResult.IsGood(error)`). - Add deterministic regression tests for both `Browse` and `BrowseNext`, adapted to the `master378` server API. ### Notes The upstream commit targets the v2.0 APIs (`ArrayOf`/`ByteString`); the source fix and tests here were adapted to the `master378` API (`byte[]` continuation points, `BrowseResultCollection`/`ByteStringCollection`, 4-arg `OperationContext` constructor). Both new tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes
BrowseandBrowseNextso they do not return a usable continuation point when the server reportsBadNoContinuationPoints, and adds regression tests that cover both code paths.Problem
According to OPC UA Part 4, if the server cannot allocate another continuation point for a
BrowseorBrowseNextoperation, the result should beBad_NoContinuationPoints. In that case, the server must not also return a continuation point that the client can continue to use.Previously,
MasterNodeManager.FetchReferencesAsync()could returnBadNoContinuationPointswhile still passing a live continuation point object back to its callers.BrowseAsync()then copied that continuation point into the result, producing a contradictory response withStatusCode = BadNoContinuationPointsand a non-emptyContinuationPoint.BrowseNextAsync()went further and overwrote the error withGoodwhenever a continuation point object was still present. That behavior hides continuation-point quota exhaustion and breaks the expected paging semantics.Changes
FetchReferencesAsync()hits the no-continuation-points path.BrowseandBrowseNextresults when the operation completed successfully.BrowseandBrowseNextreturnBadNoContinuationPointswithout a continuation point when the continuation-point quota is exhausted.