fix(driver-twincat): resolve Medium code-review finding (Driver.TwinCAT-010)
Replace yield break with cancellationToken.ThrowIfCancellationRequested() in BrowseSymbolsAsync so a cancelled browse propagates as OperationCanceledException instead of silently completing with a partial symbol set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -272,7 +272,7 @@ symbol set with no indication it was truncated. The `SymbolLoaderFactory.Create`
|
||||
`yield break` so a cancelled browse surfaces as cancellation, not as a successful but partial
|
||||
discovery.
|
||||
|
||||
**Resolution:** _(open)_
|
||||
**Resolution:** Resolved 2026-05-22 — replaced `yield break` with `cancellationToken.ThrowIfCancellationRequested()` in the `foreach` loop so a cancelled browse propagates as `OperationCanceledException`, matching the `DiscoverAsync` expectation.
|
||||
|
||||
### Driver.TwinCAT-011
|
||||
|
||||
|
||||
@@ -244,7 +244,12 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
|
||||
|
||||
foreach (ISymbol symbol in loader.Symbols)
|
||||
{
|
||||
if (cancellationToken.IsCancellationRequested) yield break;
|
||||
// ThrowIfCancellationRequested — not yield break — so a cancelled browse propagates
|
||||
// as OperationCanceledException rather than a silent clean completion. DiscoverAsync
|
||||
// has an explicit catch(OperationCanceledException){ throw; } to surface this
|
||||
// distinctly from a genuine browse failure; a yield break would let a partial
|
||||
// symbol set appear as a fully successful discovery (Driver.TwinCAT-010).
|
||||
cancellationToken.ThrowIfCancellationRequested();
|
||||
var mapped = MapSymbolTypeName(symbol.DataType?.Name);
|
||||
var readOnly = !IsSymbolWritable(symbol);
|
||||
yield return new TwinCATDiscoveredSymbol(symbol.InstancePath, mapped, readOnly);
|
||||
|
||||
Reference in New Issue
Block a user