Просмотр исходного кода

[9] fix(protocol): report the outcome of an action in its reply

Review: StatusResponse is always sent as success even if the request
fails, for example if acquire from collection were to fail for one
reason or another the client would still receive a success.
Resolution: the action is prepared before its reply is encoded, and the
reply carries whether it was refused.

The reply was encoded from a default StatusResponse before the action
ran at all, so every action reported success and its actual result was
discarded. A refused dismantle, equip, socket selection, item-state
change or Collections pull all answered exactly like one that worked.

Runs the action first and derives the status from it. Each action fills
the outcome only once it has prepared its whole transition, and consume
clears the outcome at entry, so an outcome still empty after an action
ran is that action refusing the request.

Publication order is unchanged: the prepared mutation and the
subscription are still handed to the caller for after the response is
framed. Running the action first did open one hole, which is closed
here. A response that fails to encode falls back to a correlated echo,
and that echo carries no status, so nothing may be published against it
and the outcome is cleared.

The descriptor gives the status five bits and biases logical zero to the
wire success the Client expects, so it holds no error taxonomy. One
refusal code therefore covers every reason, and the log line continues
to name the actual one.

Thomas Shields 3 недель назад
Родитель
Сommit
d60cb0ae15

+ 38 - 15
Sunrise/src/server/web_service/web_service_runtime.cpp

@@ -40,6 +40,14 @@ constexpr std::uint16_t kItemStateOpcode = 406;
 constexpr std::uint16_t kItemDismantleOpcode = 402;
 /** Web Service opcode used by Collections to create one item instance. */
 constexpr std::uint16_t kItemAcquisitionOpcode = 1820;
+/** The mutation variant's first alternative is the empty one, so index zero prepared nothing. */
+constexpr std::size_t kNoMutation = 0;
+/**
+ * Logical status of a refused action. The descriptor biases logical zero to the wire success the
+ * Client expects, so any other logical value reports a refusal. Its five bits hold no error
+ * taxonomy, so one code covers every reason and the log line names the actual one.
+ */
+constexpr std::int32_t kRefusedStatus = 1;
 
 /**
  * Logs which Web Service opcode arrived and a bounded payload trace. One svc-10 frame looks like
@@ -125,11 +133,12 @@ bool consume(std::span<const std::byte> request,
 }
 
 /**
- * Parses one request, encodes its response, and publishes checked side effects last.
+ * Parses one request, prepares any action it names, and encodes the reply that reports it.
  * @param request Whole decrypted svc-10 body.
  * @param response Svc-11 response-body storage owned by the caller.
  * @param written Gets the encoded response-body size, or zero when the header does not parse.
- * @param outcome Gets a valid family selector only after the response is encoded.
+ * @param outcome Gets the prepared action for the caller to publish, and is left empty when
+ * the action was refused or the reply could not be encoded.
  * @return False only when the envelope header does not parse.
  */
 bool consume(std::span<const std::byte> request,
@@ -197,20 +206,13 @@ bool consume(std::span<const std::byte> request,
         message.opcode == middleware::web_service::messages::opcode206::kOpcode
         && middleware::web_service::messages::opcode206::parse_request(message, subscription);
 
-    middleware::web_service::ResponseShape shape{};
-    resolve_response_shape(message.opcode, shape);
-    if (!middleware::web_service::encode_response(
-            message, shape, middleware::web_service::StatusResponse{}, response, written)) {
-        return encode_echo(message, response, written);
-    }
-    if (subscribes) {
-        // Publish the subscription only after its correlated response is complete.
-        outcome.hasSubscription = true;
-        outcome.subscription = subscription;
-        return true;
-    }
+    // The action runs before its reply is encoded, because the reply reports whether it worked.
+    // Each action fills the outcome only once it has prepared its whole transition, so an outcome
+    // still empty after one ran is that action refusing the request. Nothing is published here:
+    // both the prepared mutation and the subscription are handed back for the caller to publish
+    // once the whole response is framed.
+    bool dispatched = true;
     if (message.opcode == middleware::web_service::messages::opcode504::kOpcode) {
-        // The selection is State, not a response field, so it publishes after the reply encodes.
         select_character(message, outcome);
     } else if (message.opcode == kItemDismantleOpcode) {
         dismantle_item(message, outcome);
@@ -226,6 +228,27 @@ bool consume(std::span<const std::byte> request,
         mutate_item_state(message, outcome);
     } else if (message.opcode == kItemAcquisitionOpcode) {
         acquire_item(message, outcome);
+    } else {
+        dispatched = false;
+    }
+    const bool prepared =
+        outcome.hasSelectedCharacter || outcome.mutation.index() != kNoMutation;
+
+    middleware::web_service::ResponseShape shape{};
+    resolve_response_shape(message.opcode, shape);
+    middleware::web_service::StatusResponse status{};
+    if (dispatched && !prepared) {
+        status.code = kRefusedStatus;
+    }
+    if (!middleware::web_service::encode_response(message, shape, status, response, written)) {
+        // The echo carries no status, so nothing may be published against it.
+        outcome = {};
+        return encode_echo(message, response, written);
+    }
+    if (subscribes) {
+        // Publish the subscription only after its correlated response is complete.
+        outcome.hasSubscription = true;
+        outcome.subscription = subscription;
     }
     return true;
 }

+ 2 - 1
Sunrise/src/server/web_service/web_service_runtime.h

@@ -82,7 +82,8 @@ void report_socket_plug_response(const middleware::web_service::Message& message
  * @param request Whole decrypted svc-10 body.
  * @param response Svc-11 response-body storage owned by the caller.
  * @param written Gets the encoded response-body size, or zero when the header does not parse.
- * @param outcome Gets a valid family selector only after the response is encoded.
+ * @param outcome Gets the prepared action for the caller to publish, and is left empty when
+ * the action was refused or the reply could not be encoded.
  * @return False only when the envelope header does not parse.
  */
 [[nodiscard]] bool consume(std::span<const std::byte> request,