From b8bb41979bb6a76d64f77ea5d932ef5f0fc9795d Mon Sep 17 00:00:00 2001
From: Alberto Ricart <alberto@synadia.com>
Date: Mon, 18 Nov 2024 19:03:19 -0500
Subject: [PATCH] feat(core): subscriptions `closed` promise can now resolve to
 void or Error - if error the subscription was closed due to an error. Note
 that it doesn't reject, but rather resolves to the Error. (#146)

Signed-off-by: Alberto Ricart <alberto@synadia.com>
---
 core/src/core.ts        |  9 +++++++--
 core/src/protocol.ts    | 14 +++++++-------
 core/tests/auth_test.ts |  8 +++++++-
 migration.md            |  2 ++
 4 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/core/src/core.ts b/core/src/core.ts
index 5d01583c..f2903ae2 100644
--- a/core/src/core.ts
+++ b/core/src/core.ts
@@ -579,8 +579,13 @@ export function syncIterator<T>(src: AsyncIterable<T>): SyncIterator<T> {
  * Basic interface to a Subscription type
  */
 export interface Subscription extends AsyncIterable<Msg> {
-  /** A promise that resolves when the subscription closes */
-  closed: Promise<void>;
+  /**
+   * A promise that resolves when the subscription closes. If the promise
+   * resolves to an error, the subscription was closed because of an error
+   * typically a permissions error. Note that this promise doesn't reject, but
+   * rather resolves to void (no error) or an Error
+   */
+  closed: Promise<void | Error>;
 
   /**
    * Stop the subscription from receiving messages. You can optionally
diff --git a/core/src/protocol.ts b/core/src/protocol.ts
index 9553cc30..e8f327a8 100644
--- a/core/src/protocol.ts
+++ b/core/src/protocol.ts
@@ -141,7 +141,7 @@ export class SubscriptionImpl extends QueuedIteratorImpl<Msg>
   timer?: Timeout<void>;
   info?: unknown;
   cleanupFn?: (sub: Subscription, info?: unknown) => void;
-  closed: Deferred<void>;
+  closed: Deferred<void | Error>;
   requestSubject?: string;
   slow?: SlowNotifier;
 
@@ -156,7 +156,7 @@ export class SubscriptionImpl extends QueuedIteratorImpl<Msg>
     this.subject = subject;
     this.draining = false;
     this.noIterator = typeof opts.callback === "function";
-    this.closed = deferred();
+    this.closed = deferred<void | Error>();
 
     const asyncTraces = !(protocol.options?.noAsyncTraces || false);
 
@@ -178,8 +178,8 @@ export class SubscriptionImpl extends QueuedIteratorImpl<Msg>
     if (!this.noIterator) {
       // cleanup - they used break or return from the iterator
       // make sure we clean up, if they didn't call unsub
-      this.iterClosed.then(() => {
-        this.closed.resolve();
+      this.iterClosed.then((err: void | Error) => {
+        this.closed.resolve(err);
         this.unsubscribe();
       });
     }
@@ -203,7 +203,7 @@ export class SubscriptionImpl extends QueuedIteratorImpl<Msg>
     }
   }
 
-  close(): void {
+  close(err?: Error): void {
     if (!this.isClosed()) {
       this.cancelTimeout();
       const fn = () => {
@@ -215,7 +215,7 @@ export class SubscriptionImpl extends QueuedIteratorImpl<Msg>
             // ignoring
           }
         }
-        this.closed.resolve();
+        this.closed.resolve(err);
       };
 
       if (this.noIterator) {
@@ -350,7 +350,7 @@ export class Subscriptions {
     }
     if (sub) {
       sub.callback(err, {} as Msg);
-      sub.close();
+      sub.close(err);
       this.subs.delete(sub.sid);
       return sub !== this.mux;
     }
diff --git a/core/tests/auth_test.ts b/core/tests/auth_test.ts
index c7824686..0ad073bb 100644
--- a/core/tests/auth_test.ts
+++ b/core/tests/auth_test.ts
@@ -945,6 +945,9 @@ Deno.test("auth - perm sub iterator error", async () => {
     `Permissions Violation for Subscription to "q"`,
   );
 
+  const err = await sub.closed;
+  assertEquals(err instanceof errors.PermissionViolationError, true);
+
   await cleanup(ns, nc);
 });
 
@@ -967,7 +970,7 @@ Deno.test("auth - perm error is not in lastError", async () => {
   assertEquals(nci.protocol.lastError, undefined);
 
   const d = deferred<Error | null>();
-  nc.subscribe("q", {
+  const sub = nc.subscribe("q", {
     callback: (err) => {
       d.resolve(err);
     },
@@ -978,6 +981,9 @@ Deno.test("auth - perm error is not in lastError", async () => {
   assert(err instanceof errors.PermissionViolationError);
   assert(nci.protocol.lastError === undefined);
 
+  const err2 = await sub.closed;
+  assert(err2 instanceof errors.PermissionViolationError);
+
   await cleanup(ns, nc);
 });
 
diff --git a/migration.md b/migration.md
index bbd92b0b..47faf4e0 100644
--- a/migration.md
+++ b/migration.md
@@ -97,6 +97,8 @@ these modules for cross-runtime consumption.
   data and are easier to use from different modules, since you can provide the
   string name of the type. For more information see
   [Lifecycle and Informational and Events](core/README.md#lifecycle-and-informational-events)
+- Subscription#closed now resolves to void or an Error (it doesn't throw). The
+  error is the reason why the subscription closed.
 
 ## Changes in JetStream