feat(inbound-api): machine-readable error codes per spec (incl. SITE_UNREACHABLE) + 415 for non-JSON bodies
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -51,6 +51,21 @@ public static class EndpointExtensions
|
||||
/// </summary>
|
||||
private const string UnauthorizedMessage = "Invalid or missing API key";
|
||||
|
||||
/// <summary>
|
||||
/// C3: builds the canonical inbound-API error body — <c>{ "error": message, "code":
|
||||
/// code }</c> — at the given HTTP status. Every error return in
|
||||
/// <see cref="HandleInboundApiRequest"/> routes through this helper so the
|
||||
/// machine-readable <c>code</c> field (the audience is external integrators) is
|
||||
/// present and consistent across all failure branches. The <c>error</c> field is
|
||||
/// unchanged, so callers that only read it are unaffected (additive field).
|
||||
/// </summary>
|
||||
/// <param name="code">The machine-readable error code (e.g. <c>UNAUTHORIZED</c>, <c>SITE_UNREACHABLE</c>).</param>
|
||||
/// <param name="message">The human-readable, caller-safe error message.</param>
|
||||
/// <param name="status">The HTTP status code for the response.</param>
|
||||
/// <returns>A JSON result carrying the error + code at the given status.</returns>
|
||||
private static IResult Error(string code, string message, int status) =>
|
||||
Results.Json(new { error = message, code }, statusCode: status);
|
||||
|
||||
/// <summary>Registers the <c>POST /api/{methodName}</c> inbound API endpoint with the active-node gate and body-size filter applied.</summary>
|
||||
/// <param name="endpoints">The route builder to add the endpoint to.</param>
|
||||
/// <returns>The same <paramref name="endpoints"/> instance for chaining.</returns>
|
||||
@@ -108,9 +123,7 @@ public static class EndpointExtensions
|
||||
"Inbound API auth failure for method {Method}: {Failure} (status 401)",
|
||||
methodName, verification.Failure);
|
||||
|
||||
return Results.Json(
|
||||
new { error = UnauthorizedMessage },
|
||||
statusCode: StatusCodes.Status401Unauthorized);
|
||||
return Error("UNAUTHORIZED", UnauthorizedMessage, StatusCodes.Status401Unauthorized);
|
||||
}
|
||||
|
||||
var identity = verification.Identity!;
|
||||
@@ -150,9 +163,7 @@ public static class EndpointExtensions
|
||||
"Inbound API authz failure for method {Method}: not approved (status 403)",
|
||||
methodName);
|
||||
|
||||
return Results.Json(
|
||||
new { error = NotApprovedMessage },
|
||||
statusCode: StatusCodes.Status403Forbidden);
|
||||
return Error("NOT_APPROVED", NotApprovedMessage, StatusCodes.Status403Forbidden);
|
||||
}
|
||||
|
||||
// Telemetry follow-on: count this inbound request against the resolved,
|
||||
@@ -168,6 +179,20 @@ public static class EndpointExtensions
|
||||
httpContext.Items[AuditWriteMiddleware.AuditActorItemKey] =
|
||||
identity.DisplayName;
|
||||
|
||||
// S9: a body sent with an explicit non-JSON Content-Type is a media-type
|
||||
// mismatch, not malformed JSON — reject it with 415 rather than a misleading
|
||||
// 400 "invalid JSON". A body with NO Content-Type header is still parsed
|
||||
// leniently as JSON below (preserving existing callers). The sniff is
|
||||
// ordinal-ignore-case so "application/JSON" etc. are treated as JSON.
|
||||
if (httpContext.Request.ContentLength > 0
|
||||
&& !string.IsNullOrEmpty(httpContext.Request.ContentType)
|
||||
&& httpContext.Request.ContentType.Contains("json", StringComparison.OrdinalIgnoreCase) == false)
|
||||
{
|
||||
return Error(
|
||||
"UNSUPPORTED_MEDIA_TYPE", "Content-Type must be application/json",
|
||||
StatusCodes.Status415UnsupportedMediaType);
|
||||
}
|
||||
|
||||
// Deserialize and validate parameters
|
||||
JsonElement? body = null;
|
||||
try
|
||||
@@ -188,9 +213,7 @@ public static class EndpointExtensions
|
||||
}
|
||||
catch (JsonException)
|
||||
{
|
||||
return Results.Json(
|
||||
new { error = "Invalid JSON in request body" },
|
||||
statusCode: 400);
|
||||
return Error("INVALID_JSON", "Invalid JSON in request body", StatusCodes.Status400BadRequest);
|
||||
}
|
||||
|
||||
// Thread the JSON-Schema $ref resolution seam into parameter
|
||||
@@ -209,9 +232,9 @@ public static class EndpointExtensions
|
||||
var paramResult = ParameterValidator.Validate(body, method.ParameterDefinitions, resolveRef);
|
||||
if (!paramResult.IsValid)
|
||||
{
|
||||
return Results.Json(
|
||||
new { error = paramResult.ErrorMessage },
|
||||
statusCode: 400);
|
||||
return Error(
|
||||
"VALIDATION_FAILED", paramResult.ErrorMessage ?? "Parameter validation failed",
|
||||
StatusCodes.Status400BadRequest);
|
||||
}
|
||||
|
||||
// Execute the method's script
|
||||
@@ -250,9 +273,13 @@ public static class EndpointExtensions
|
||||
"Inbound API script failure for method {Method}: {Error}",
|
||||
methodName, scriptResult.ErrorMessage);
|
||||
|
||||
return Results.Json(
|
||||
new { error = scriptResult.ErrorMessage ?? "Internal server error" },
|
||||
statusCode: 500);
|
||||
// C3: propagate the executor's machine-readable code (TIMEOUT,
|
||||
// SITE_UNREACHABLE, SCRIPT_COMPILE_FAILED, …); an unclassified failure
|
||||
// falls back to the SCRIPT_ERROR catch-all.
|
||||
return Error(
|
||||
scriptResult.ErrorCode ?? "SCRIPT_ERROR",
|
||||
scriptResult.ErrorMessage ?? "Internal server error",
|
||||
StatusCodes.Status500InternalServerError);
|
||||
}
|
||||
|
||||
// Return the script result as JSON
|
||||
|
||||
@@ -56,7 +56,7 @@ public sealed class InboundApiEndpointFilter : IEndpointFilter
|
||||
_logger.LogWarning(
|
||||
"Inbound API request rejected — this node is a standby (not the active central node)");
|
||||
return Results.Json(
|
||||
new { error = "Inbound API is only available on the active central node" },
|
||||
new { error = "Inbound API is only available on the active central node", code = "STANDBY_NODE" },
|
||||
statusCode: StatusCodes.Status503ServiceUnavailable);
|
||||
}
|
||||
|
||||
@@ -70,7 +70,7 @@ public sealed class InboundApiEndpointFilter : IEndpointFilter
|
||||
"Inbound API request rejected — body length {Length} exceeds limit {Limit}",
|
||||
declaredLength, maxBytes);
|
||||
return Results.Json(
|
||||
new { error = "Request body too large" },
|
||||
new { error = "Request body too large", code = "BODY_TOO_LARGE" },
|
||||
statusCode: StatusCodes.Status413PayloadTooLarge);
|
||||
}
|
||||
|
||||
|
||||
@@ -384,7 +384,7 @@ public class InboundScriptExecutor
|
||||
// Fast-fail only when the CURRENT script text is the one already known bad.
|
||||
if (_knownBadMethods.TryGetValue(method.Name, out var badScript)
|
||||
&& string.Equals(badScript, method.Script, StringComparison.Ordinal))
|
||||
return new InboundScriptResult(false, null, "Script compilation failed for this method");
|
||||
return new InboundScriptResult(false, null, "Script compilation failed for this method", "SCRIPT_COMPILE_FAILED");
|
||||
|
||||
// Lazy compile (handles methods created after startup, and stale/known-bad
|
||||
// recompiles). Compile outside the cache so a failed compile is not stored,
|
||||
@@ -396,7 +396,7 @@ public class InboundScriptExecutor
|
||||
// Routed through TryRecordBadMethod so the
|
||||
// cache is bounded under a flood of unique method names.
|
||||
TryRecordBadMethod(method.Name, method.Script ?? string.Empty);
|
||||
return new InboundScriptResult(false, null, "Script compilation failed for this method");
|
||||
return new InboundScriptResult(false, null, "Script compilation failed for this method", "SCRIPT_COMPILE_FAILED");
|
||||
}
|
||||
|
||||
_knownBadMethods.TryRemove(method.Name, out _);
|
||||
@@ -448,17 +448,25 @@ public class InboundScriptExecutor
|
||||
if (timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested)
|
||||
{
|
||||
_logger.LogWarning("Script execution timed out for method {Method}", method.Name);
|
||||
return new InboundScriptResult(false, null, "Script execution timed out");
|
||||
return new InboundScriptResult(false, null, "Script execution timed out", "TIMEOUT");
|
||||
}
|
||||
|
||||
_logger.LogDebug("Inbound API request for method {Method} cancelled by client", method.Name);
|
||||
return new InboundScriptResult(false, null, "Request cancelled by client");
|
||||
}
|
||||
catch (SiteUnreachableException)
|
||||
{
|
||||
// C3: a routed Route.To(...).Call(...) reached a site that could not be
|
||||
// contacted. Surface the machine-readable SITE_UNREACHABLE code (spec
|
||||
// Component-InboundAPI failure body) instead of the generic SCRIPT_ERROR.
|
||||
_logger.LogWarning("Routed site unreachable for method {Method}", method.Name);
|
||||
return new InboundScriptResult(false, null, "Site unreachable", "SITE_UNREACHABLE");
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogError(ex, "Script execution failed for method {Method}", method.Name);
|
||||
// Safe error message, no internal details
|
||||
return new InboundScriptResult(false, null, "Internal script error");
|
||||
return new InboundScriptResult(false, null, "Internal script error", "SCRIPT_ERROR");
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -537,7 +545,17 @@ public class InboundScriptContext
|
||||
/// <summary>
|
||||
/// Result of executing an inbound API script.
|
||||
/// </summary>
|
||||
/// <param name="Success">Whether the script executed successfully.</param>
|
||||
/// <param name="ResultJson">The serialized script return value on success; null otherwise.</param>
|
||||
/// <param name="ErrorMessage">A sanitized, caller-safe error message on failure; null on success.</param>
|
||||
/// <param name="ErrorCode">
|
||||
/// C3: an additive, machine-readable error code the endpoint surfaces in the failure
|
||||
/// body's <c>code</c> field (e.g. <c>TIMEOUT</c>, <c>SITE_UNREACHABLE</c>,
|
||||
/// <c>SCRIPT_COMPILE_FAILED</c>, <c>SCRIPT_ERROR</c>). Null when the executor does not
|
||||
/// classify the failure; the endpoint then falls back to <c>SCRIPT_ERROR</c>.
|
||||
/// </param>
|
||||
public record InboundScriptResult(
|
||||
bool Success,
|
||||
string? ResultJson,
|
||||
string? ErrorMessage);
|
||||
string? ErrorMessage,
|
||||
string? ErrorCode = null);
|
||||
|
||||
@@ -4,6 +4,22 @@ 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
|
||||
/// plain <see cref="InvalidOperationException"/>.
|
||||
/// </summary>
|
||||
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>
|
||||
/// Route.To() helper for cross-site calls from inbound API scripts.
|
||||
/// Resolves instance to site, routes via <see cref="IInstanceRouter"/>, blocks until
|
||||
@@ -170,7 +186,7 @@ public class RouteTarget
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
throw RoutingFailure(
|
||||
response.ErrorMessage ?? "Remote script call failed");
|
||||
}
|
||||
|
||||
@@ -218,7 +234,7 @@ public class RouteTarget
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
throw RoutingFailure(
|
||||
response.ErrorMessage ?? "Remote attribute read failed");
|
||||
}
|
||||
|
||||
@@ -282,7 +298,7 @@ public class RouteTarget
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
throw RoutingFailure(
|
||||
response.ErrorMessage ?? "Remote attribute wait failed");
|
||||
}
|
||||
|
||||
@@ -333,7 +349,7 @@ public class RouteTarget
|
||||
|
||||
if (!response.Success)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
throw RoutingFailure(
|
||||
response.ErrorMessage ?? "Remote attribute write failed");
|
||||
}
|
||||
}
|
||||
@@ -350,10 +366,30 @@ public class RouteTarget
|
||||
var siteId = await _instanceLocator.GetSiteIdForInstanceAsync(_instanceCode, cancellationToken);
|
||||
if (siteId == null)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
// 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.
|
||||
throw RoutingFailure(
|
||||
$"Instance '{_instanceCode}' not found or has no assigned site");
|
||||
}
|
||||
|
||||
return siteId;
|
||||
}
|
||||
|
||||
/// <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"/>.
|
||||
/// </summary>
|
||||
private static InvalidOperationException RoutingFailure(string message) =>
|
||||
IsUnreachable(message)
|
||||
? new SiteUnreachableException(message)
|
||||
: new InvalidOperationException(message);
|
||||
|
||||
private static bool IsUnreachable(string message) =>
|
||||
message.Contains("unreachable", StringComparison.OrdinalIgnoreCase)
|
||||
|| message.Contains("no contact", StringComparison.OrdinalIgnoreCase)
|
||||
|| message.Contains("no-contact", StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user