More handling of errors. This now improves a bit more the error handling situation (#555)

Author: slinkydeveloper

packages/restate-sdk-cloudflare-workers/patches/vm/sdk_shared_core_wasm_bindings_bg.wasm

@@ -101,7 +101,14 @@ export type {
 } from "@restatedev/restate-sdk-core";
 
 export type { RestateEndpoint, RestateEndpointBase } from "./endpoint.js";
-export { RestateError, TerminalError, TimeoutError } from "./types/errors.js";
+export {
+  /**
+   * @deprecated YOU MUST NOT USE THIS TYPE
+   */
+  RestateError,
+  TerminalError,
+  TimeoutError,
+} from "./types/errors.js";
 export type {
   LoggerTransport,
   LogMetadata,

packages/restate-sdk/src/common_api.ts

@@ -36,8 +36,8 @@ import {
 import {
   ensureError,
   INTERNAL_ERROR_CODE,
+  logError,
   RestateError,
-  SUSPENDED_ERROR_CODE,
   TerminalError,
   UNKNOWN_ERROR_CODE,
 } from "./types/errors.js";
@@ -125,7 +125,7 @@ export class ContextImpl implements ObjectContext, WorkflowContext {
       ),
       this.outputPump,
       this.runClosuresTracker,
-      this.handleInvocationEndError.bind(this)
+      this.promiseExecutorErrorCallback.bind(this)
     );
   }
 
@@ -696,21 +696,11 @@ export class ContextImpl implements ObjectContext, WorkflowContext {
     );
   }
 
-  handleInvocationEndError(
-    e: unknown,
-    notify_vm_error: (vm: vm.WasmVM, error: Error) => void = (vm, error) => {
-      vm.notify_error(error.message, error.stack);
-    }
-  ) {
+  promiseExecutorErrorCallback(e: unknown) {
     if (e instanceof AsyncCompleterError) {
       const cause = ensureError(e.cause);
-
+      logError(this.vmLogger, e.cause);
       // Special handling for this one!
-      this.vmLogger.warn(
-        "Error when processing a Restate context operation.\n",
-        cause
-      );
-
       this.coreVm.notify_error_for_specific_command(
         cause.message,
         cause.stack,
@@ -720,21 +710,30 @@ export class ContextImpl implements ObjectContext, WorkflowContext {
       );
     } else {
       const error = ensureError(e);
-      if (
-        !(error instanceof RestateError) ||
-        error.code !== SUSPENDED_ERROR_CODE
-      ) {
-        this.vmLogger.warn(
-          "Error when processing a Restate context operation.\n",
-          error
-        );
+      logError(this.vmLogger, error);
+      if (!(error instanceof RestateError)) {
+        // Notify error
+        this.coreVm.notify_error(error.message, error.stack);
       }
-      notify_vm_error(this.coreVm, error);
     }
 
     // From now on, no progress will be made.
     this.invocationEndPromise.resolve();
   }
+
+  handleInvocationEndError(
+    e: unknown,
+    notify_vm_error: (vm: vm.WasmVM, error: Error) => void = (vm, error) => {
+      vm.notify_error(error.message, error.stack);
+    }
+  ) {
+    const error = ensureError(e);
+    logError(this.vmLogger, error);
+    notify_vm_error(this.coreVm, error);
+
+    // From now on, no progress will be made.
+    this.invocationEndPromise.resolve();
+  }
 }
 
 function unpackRunParameters<T>(

packages/restate-sdk/src/context_impl.ts

@@ -11,8 +11,8 @@
 
 import {
   ensureError,
+  logError,
   RestateError,
-  SUSPENDED_ERROR_CODE,
   TerminalError,
 } from "../../types/errors.js";
 import type { ProtocolMode } from "../../types/discovery.js";
@@ -377,12 +377,7 @@ export class GenericHandler implements RestateHandler {
         })
         .catch((e) => {
           const error = ensureError(e);
-          if (
-            !(error instanceof RestateError) ||
-            error.code !== SUSPENDED_ERROR_CODE
-          ) {
-            vmLogger.warn("Invocation completed with an error.\n", error);
-          }
+          logError(vmLogger, error);
 
           if (error instanceof TerminalError) {
             coreVm.sys_write_output_failure({

packages/restate-sdk/src/endpoint/handlers/generic.ts

@@ -446,7 +446,7 @@ export class PromisesExecutor {
         }
 
         // Recursion
-        await this.doProgress(restatePromise);
+        await this.doProgressInner(restatePromise);
       } catch (e) {
         // Not good, this is a retryable error.
         this.errorCallback(e);

packages/restate-sdk/src/endpoint/handlers/vm/sdk_shared_core_wasm_bindings.js

@@ -17,6 +17,7 @@ export const CANCEL_ERROR_CODE = 409;
 export const UNKNOWN_ERROR_CODE = 500;
 
 // From shared core!
+export const CLOSED_ERROR_CODE = 598;
 export const SUSPENDED_ERROR_CODE = 599;
 
 export function ensureError(e: unknown): Error {
@@ -40,6 +41,25 @@ export function ensureError(e: unknown): Error {
   return new Error("Non-Error value: " + msg);
 }
 
+export function logError(log: Console, e: unknown) {
+  if (e instanceof RestateError) {
+    if (e.code === SUSPENDED_ERROR_CODE) {
+      log.info("Invocation suspended");
+      return;
+    } else if (e.code === CLOSED_ERROR_CODE) {
+      log.error(
+        "DANGER! The invocation is closed, but some related operation is still running. \n" +
+          "This might indicate that a RestatePromise is being awaited on an asynchronous task, outside the handler, or you're awaiting a RestatePromise inside a ctx.run.\n" +
+          "This is dangerous, and can lead the service to deadlock. Please fix the issue.\n" +
+          "Diagnostic: ",
+        e
+      );
+      return;
+    }
+  }
+  log.warn("Error when processing a Restate context operation.\n", e);
+}
+
 export class RestateError extends Error {
   public readonly code: number;
   public name = "RestateError";

packages/restate-sdk/src/promises.ts

@@ -12,7 +12,7 @@ default = ["console_error_panic_hook"]
 
 [dependencies]
 wasm-bindgen = "0.2.100"
-restate-sdk-shared-core = { git = "https://github.com/restatedev/sdk-shared-core.git", rev = "168a71b60ee37eb344d38562ff5bbec4ff6f86b9", features = ["request_identity"] }
+restate-sdk-shared-core = { git = "https://github.com/restatedev/sdk-shared-core.git", rev = "96b78a6160880cc4b34c56750dd96652e11cd81c", features = ["request_identity"] }
 console_error_panic_hook = { version = "0.1.7", optional = true }
 serde = { version = "1.0.210", features = ["derive"] }
 serde-wasm-bindgen = "0.6.5"