fix(inbound): type SITE_UNREACHABLE on AskTimeoutException at the router await instead of message-substring sniffing (plan R2-06 T5)
This commit is contained in:
@@ -124,7 +124,7 @@ The `sbk_<keyId>_<secret>` design already supports zero-downtime rotation — a
|
||||
| `BODY_TOO_LARGE` | 413 | Request body exceeded `MaxRequestBodyBytes`. |
|
||||
| `STANDBY_NODE` | 503 | Request hit a standby central node; the inbound API serves only the active node. |
|
||||
| `TIMEOUT` | 500 | The method script exceeded its execution timeout. |
|
||||
| `SITE_UNREACHABLE` | 500 | A routed `Route.To(...).Call(...)` could not reach the target site. |
|
||||
| `SITE_UNREACHABLE` | 500 | The target site did not respond to the routed request within the routing timeout (no contact / down / partitioned) — typed from the routed Ask expiring (`AskTimeoutException`), never inferred from an error-message substring. A `Success=false` response means the site answered and is classified `SCRIPT_ERROR`. |
|
||||
| `SCRIPT_COMPILE_FAILED` | 500 | The method's script failed to compile (returned consistently on every node until a compiling version is saved). |
|
||||
| `SCRIPT_ERROR` | 500 | Catch-all for any other script-execution failure (details are logged centrally, never leaked to the caller). |
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
using Akka.Actor;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||
@@ -5,12 +6,16 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||
namespace ZB.MOM.WW.ScadaBridge.InboundAPI;
|
||||
|
||||
/// <summary>
|
||||
/// C3: a routed call failed because the target site could not be reached (no contact /
|
||||
/// unreachable). Derives from <see cref="InvalidOperationException"/> so existing
|
||||
/// callers that catch the base type are unaffected, while
|
||||
/// <see cref="InboundScriptExecutor"/> catches this more specific type to surface the
|
||||
/// spec's <c>SITE_UNREACHABLE</c> error code instead of a generic <c>SCRIPT_ERROR</c>.
|
||||
/// A routing failure whose message does <em>not</em> indicate unreachability stays a
|
||||
/// C3/N3: a routed call failed because the target site never answered — the routed
|
||||
/// Ask <b>expired</b>. Unreachability is typed at the Ask boundary from
|
||||
/// <see cref="AskTimeoutException"/> (the communication layer's own signal: an
|
||||
/// unreachable/contactless site has its enveloped message dropped so the Ask times
|
||||
/// out caller-side) — it is <em>never</em> inferred from an error-message substring.
|
||||
/// Derives from <see cref="InvalidOperationException"/> so existing callers that
|
||||
/// catch the base type are unaffected, while <see cref="InboundScriptExecutor"/>
|
||||
/// catches this more specific type to surface the spec's <c>SITE_UNREACHABLE</c>
|
||||
/// error code instead of a generic <c>SCRIPT_ERROR</c>. A <c>Success=false</c>
|
||||
/// <em>response</em> means the site answered (reachable by definition) and stays a
|
||||
/// plain <see cref="InvalidOperationException"/>.
|
||||
/// </summary>
|
||||
public sealed class SiteUnreachableException : InvalidOperationException
|
||||
@@ -18,6 +23,11 @@ public sealed class SiteUnreachableException : InvalidOperationException
|
||||
/// <summary>Initializes a new <see cref="SiteUnreachableException"/> with the given message.</summary>
|
||||
/// <param name="message">The routing-level failure message that indicated the site was unreachable.</param>
|
||||
public SiteUnreachableException(string message) : base(message) { }
|
||||
|
||||
/// <summary>Initializes a new <see cref="SiteUnreachableException"/> wrapping the underlying Ask-timeout.</summary>
|
||||
/// <param name="message">The routing-level failure message.</param>
|
||||
/// <param name="innerException">The underlying <see cref="AskTimeoutException"/> from the routed Ask.</param>
|
||||
public SiteUnreachableException(string message, Exception innerException) : base(message, innerException) { }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -182,7 +192,7 @@ public class RouteTarget
|
||||
correlationId, _instanceCode, scriptName, ScriptArgs.Normalize(parameters),
|
||||
DateTimeOffset.UtcNow, _parentExecutionId);
|
||||
|
||||
var response = await _instanceRouter.RouteToCallAsync(siteId, request, token);
|
||||
var response = await RouteOrUnreachable(_instanceRouter.RouteToCallAsync(siteId, request, token));
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
@@ -230,7 +240,7 @@ public class RouteTarget
|
||||
correlationId, _instanceCode, attributeNames.ToList(), DateTimeOffset.UtcNow,
|
||||
_parentExecutionId);
|
||||
|
||||
var response = await _instanceRouter.RouteToGetAttributesAsync(siteId, request, token);
|
||||
var response = await RouteOrUnreachable(_instanceRouter.RouteToGetAttributesAsync(siteId, request, token));
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
@@ -294,7 +304,7 @@ public class RouteTarget
|
||||
AttributeValueCodec.Encode(targetValue), timeout, DateTimeOffset.UtcNow,
|
||||
_parentExecutionId);
|
||||
|
||||
var response = await _instanceRouter.RouteToWaitForAttributeAsync(siteId, request, token);
|
||||
var response = await RouteOrUnreachable(_instanceRouter.RouteToWaitForAttributeAsync(siteId, request, token));
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
@@ -345,7 +355,7 @@ public class RouteTarget
|
||||
correlationId, _instanceCode, attributeValues, DateTimeOffset.UtcNow,
|
||||
_parentExecutionId);
|
||||
|
||||
var response = await _instanceRouter.RouteToSetAttributesAsync(siteId, request, token);
|
||||
var response = await RouteOrUnreachable(_instanceRouter.RouteToSetAttributesAsync(siteId, request, token));
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
@@ -366,9 +376,9 @@ public class RouteTarget
|
||||
var siteId = await _instanceLocator.GetSiteIdForInstanceAsync(_instanceCode, cancellationToken);
|
||||
if (siteId == null)
|
||||
{
|
||||
// Route the site-resolution failure through the same classifier so a
|
||||
// message indicating the site is unreachable surfaces as SITE_UNREACHABLE;
|
||||
// a plain not-found / no-assigned-site stays a generic InvalidOperationException.
|
||||
// A null site id is a plain not-found / no-assigned-site — never
|
||||
// unreachability (nothing was routed), so it stays a generic
|
||||
// InvalidOperationException (SCRIPT_ERROR).
|
||||
throw RoutingFailure(
|
||||
$"Instance '{_instanceCode}' not found or has no assigned site");
|
||||
}
|
||||
@@ -377,19 +387,37 @@ public class RouteTarget
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// C3: classifies a routing-level failure message. When the message indicates the
|
||||
/// site could not be reached (unreachable / no contact) the failure is a
|
||||
/// <see cref="SiteUnreachableException"/> — so <see cref="InboundScriptExecutor"/>
|
||||
/// can surface the spec's <c>SITE_UNREACHABLE</c> code — otherwise a plain
|
||||
/// <see cref="InvalidOperationException"/>.
|
||||
/// N3: awaits a routed Ask and TYPES unreachability. AskTimeoutException means
|
||||
/// the site never answered (no ClusterClient contact, site down, partition) —
|
||||
/// the communication layer's own typed signal (CentralCommunicationActor drops
|
||||
/// envelopes for contactless sites so the Ask expires caller-side) — and
|
||||
/// surfaces as SiteUnreachableException so the executor maps it to the spec's
|
||||
/// SITE_UNREACHABLE code. A Success=false RESPONSE means the site answered and
|
||||
/// is therefore reachable; it flows through RoutingFailure as a plain
|
||||
/// InvalidOperationException (SCRIPT_ERROR).
|
||||
/// </summary>
|
||||
private static InvalidOperationException RoutingFailure(string message) =>
|
||||
IsUnreachable(message)
|
||||
? new SiteUnreachableException(message)
|
||||
: new InvalidOperationException(message);
|
||||
private static async Task<TResponse> RouteOrUnreachable<TResponse>(Task<TResponse> routedAsk)
|
||||
{
|
||||
try
|
||||
{
|
||||
return await routedAsk;
|
||||
}
|
||||
catch (AskTimeoutException ex)
|
||||
{
|
||||
throw new SiteUnreachableException(
|
||||
"Site did not respond to the routed request within the routing timeout — unreachable or not connected.",
|
||||
ex);
|
||||
}
|
||||
}
|
||||
|
||||
private static bool IsUnreachable(string message) =>
|
||||
message.Contains("unreachable", StringComparison.OrdinalIgnoreCase)
|
||||
|| message.Contains("no contact", StringComparison.OrdinalIgnoreCase)
|
||||
|| message.Contains("no-contact", StringComparison.OrdinalIgnoreCase);
|
||||
/// <summary>
|
||||
/// C3/N3: a routing-level failure REPORTED BY the site or the locator. The
|
||||
/// remote end answered (or the instance simply has no site), so this is never
|
||||
/// unreachability — always a plain <see cref="InvalidOperationException"/>
|
||||
/// (SCRIPT_ERROR at the endpoint). Unreachability is typed at the Ask boundary
|
||||
/// by <c>RouteOrUnreachable</c> (AskTimeoutException → SiteUnreachableException),
|
||||
/// replacing the old message-substring sniff that a harmless rewording in the
|
||||
/// communication layer could silently defeat.
|
||||
/// </summary>
|
||||
private static InvalidOperationException RoutingFailure(string message) => new(message);
|
||||
}
|
||||
|
||||
@@ -98,19 +98,17 @@ public class InboundScriptExecutorTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RoutedSiteUnreachable_Returns500WithSiteUnreachableCode()
|
||||
public async Task RoutedAskTimeout_Returns500WithSiteUnreachableCode()
|
||||
{
|
||||
// C3: a routed call whose site is unreachable must surface the machine-readable
|
||||
// SITE_UNREACHABLE code (spec Component-InboundAPI failure body) rather than
|
||||
// collapsing into a generic SCRIPT_ERROR.
|
||||
// N3: unreachability is the Ask EXPIRING (site never answered) — typed as
|
||||
// AskTimeoutException by the communication layer — not a substring in a
|
||||
// failure message. Pre-fix this fell into the generic catch as SCRIPT_ERROR.
|
||||
var locator = Substitute.For<IInstanceLocator>();
|
||||
locator.GetSiteIdForInstanceAsync("X", Arg.Any<CancellationToken>()).Returns("SiteA");
|
||||
var router = Substitute.For<IInstanceRouter>();
|
||||
router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
|
||||
.Returns(ci => new RouteToCallResponse(
|
||||
((RouteToCallRequest)ci[1]).CorrelationId,
|
||||
Success: false, ReturnValue: null, ErrorMessage: "Site unreachable",
|
||||
DateTimeOffset.UtcNow));
|
||||
.Returns(Task.FromException<RouteToCallResponse>(
|
||||
new Akka.Actor.AskTimeoutException("Timeout after 00:00:30")));
|
||||
var route = new RouteHelper(locator, router);
|
||||
|
||||
var method = new ApiMethod("routed", "return await Route.To(\"X\").Call(\"s\");")
|
||||
@@ -123,6 +121,33 @@ public class InboundScriptExecutorTests
|
||||
Assert.Equal("SITE_UNREACHABLE", result.ErrorCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SiteRespondedFailure_MessageMentioningUnreachable_IsScriptErrorNotSiteUnreachable()
|
||||
{
|
||||
// N3 anti-sniff: a site that RESPONDED with Success=false is reachable by
|
||||
// definition — even when its error text happens to contain "unreachable".
|
||||
// Pre-fix the substring sniff misclassified this as SITE_UNREACHABLE.
|
||||
var locator = Substitute.For<IInstanceLocator>();
|
||||
locator.GetSiteIdForInstanceAsync("X", Arg.Any<CancellationToken>()).Returns("SiteA");
|
||||
var router = Substitute.For<IInstanceRouter>();
|
||||
router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
|
||||
.Returns(ci => new RouteToCallResponse(
|
||||
((RouteToCallRequest)ci[1]).CorrelationId,
|
||||
Success: false, ReturnValue: null,
|
||||
ErrorMessage: "PLC device unreachable behind the site gateway",
|
||||
DateTimeOffset.UtcNow));
|
||||
var route = new RouteHelper(locator, router);
|
||||
|
||||
var method = new ApiMethod("routed2", "return await Route.To(\"X\").Call(\"s\");")
|
||||
{ Id = 8, TimeoutSeconds = 10 };
|
||||
|
||||
var result = await _executor.ExecuteAsync(
|
||||
method, new Dictionary<string, object?>(), route, TimeSpan.FromSeconds(10));
|
||||
|
||||
Assert.False(result.Success);
|
||||
Assert.Equal("SCRIPT_ERROR", result.ErrorCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task HandlerTimesOut_ReturnsTimeoutError()
|
||||
{
|
||||
|
||||
@@ -595,4 +595,30 @@ public class RouteHelperTests
|
||||
Assert.NotNull(captured);
|
||||
Assert.Equal(inboundExecutionId, captured!.ParentExecutionId);
|
||||
}
|
||||
|
||||
// --- N3: typed unreachability at the Ask boundary ---
|
||||
|
||||
[Fact]
|
||||
public async Task Call_AskTimeout_ThrowsSiteUnreachableException()
|
||||
{
|
||||
SiteResolves("inst", "SiteA");
|
||||
_router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
|
||||
.Returns(Task.FromException<RouteToCallResponse>(
|
||||
new Akka.Actor.AskTimeoutException("Timeout after 00:00:30")));
|
||||
|
||||
await Assert.ThrowsAsync<SiteUnreachableException>(
|
||||
() => CreateHelper().To("inst").Call("s"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetAttributes_AskTimeout_ThrowsSiteUnreachableException()
|
||||
{
|
||||
SiteResolves("inst", "SiteA");
|
||||
_router.RouteToGetAttributesAsync("SiteA", Arg.Any<RouteToGetAttributesRequest>(), Arg.Any<CancellationToken>())
|
||||
.Returns(Task.FromException<RouteToGetAttributesResponse>(
|
||||
new Akka.Actor.AskTimeoutException("Timeout after 00:00:30")));
|
||||
|
||||
await Assert.ThrowsAsync<SiteUnreachableException>(
|
||||
() => CreateHelper().To("inst").GetAttributes(new[] { "a" }));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user