From 500e31d6a35b8da02abd9de81827af33f71a9eac Mon Sep 17 00:00:00 2001
From: Tomas Valenta <valenta.and.thomas@gmail.com>
Date: Mon, 25 Nov 2024 14:20:27 -0800
Subject: [PATCH] Improve pause/resume SDK return types and clarify usage

---
 packages/js-sdk/src/sandbox/index.ts      | 38 ++++++++++++++++++++---
 packages/js-sdk/src/sandbox/sandboxApi.ts | 26 ++++++++++------
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/packages/js-sdk/src/sandbox/index.ts b/packages/js-sdk/src/sandbox/index.ts
index 1faa4018e..e16e9ec0a 100644
--- a/packages/js-sdk/src/sandbox/index.ts
+++ b/packages/js-sdk/src/sandbox/index.ts
@@ -217,14 +217,25 @@ export class Sandbox extends SandboxApi {
     return sbx
   }
 
+  /**
+   * Resume the sandbox.
+   * 
+   * The **default sandbox timeout of 300 seconds** ({@link Sandbox.defaultSandboxTimeoutMs}) will be used for the resumed sandbox.
+   * If you pass a custom timeout in the `opts` parametervia {@link SandboxOpts.timeoutMs} property, it will be used instead.
+   *
+   * @param sandboxId sandbox ID.
+   * @param opts connection options.
+   *
+   * @returns sandbox instance.
+   */
   static async resume<S extends typeof Sandbox>(
     this: S,
     sandboxId: string,
     opts?: Pick<SandboxOpts, 'requestTimeoutMs' | 'timeoutMs'>
   ): Promise<InstanceType<S>> {
-    const id = await Sandbox.resumeSandbox(sandboxId, opts?.timeoutMs ?? this.defaultSandboxTimeoutMs, opts)
+    await Sandbox.resumeSandbox(sandboxId, opts?.timeoutMs ?? this.defaultSandboxTimeoutMs, opts)
 
-    return this.connect(id, opts)
+    return await this.connect(sandboxId, opts)
   }
 
   /**
@@ -326,16 +337,33 @@ export class Sandbox extends SandboxApi {
     await Sandbox.kill(this.sandboxId, { ...this.connectionConfig, ...opts })
   }
 
-  async pause(opts?: Pick<SandboxOpts, 'requestTimeoutMs'>): Promise<string> {
+  /**
+   * Pause the sandbox.
+   *
+   * @param opts connection options.
+   *
+   * @returns `true` if the sandbox got paused, `false` if the sandbox was already paused.
+   */
+  async pause(opts?: Pick<SandboxOpts, 'requestTimeoutMs'>): Promise<boolean> {
     if (this.connectionConfig.debug) {
       // Skip pausing in debug mode
-      return this.sandboxId
+      return true
     }
 
     return await Sandbox.pause(this.sandboxId, { ...this.connectionConfig, ...opts })
   }
 
-  async resume(opts?: Pick<SandboxOpts, 'requestTimeoutMs'>) {
+  /**
+   * Resume the sandbox.
+   * 
+   * The **default sandbox timeout of 300 seconds** ({@link Sandbox.defaultSandboxTimeoutMs}) will be used for the resumed sandbox.
+   * If you pass a custom timeout in the `opts` parameter via {@link SandboxOpts.timeoutMs} property, it will be used instead.
+   *
+   * @param opts connection options.
+   *
+   * @returns sandbox instance.
+   */
+  async resume(opts?: Pick<SandboxOpts, 'requestTimeoutMs'>): Promise<this> {
     await Sandbox.resume(this.sandboxId, { ...this.connectionConfig, ...opts })
 
     return this
diff --git a/packages/js-sdk/src/sandbox/sandboxApi.ts b/packages/js-sdk/src/sandbox/sandboxApi.ts
index 2f9ab8f17..d42f425c5 100644
--- a/packages/js-sdk/src/sandbox/sandboxApi.ts
+++ b/packages/js-sdk/src/sandbox/sandboxApi.ts
@@ -1,7 +1,7 @@
 import { ApiClient, components, handleApiError } from '../api'
 import { ConnectionConfig, ConnectionOpts } from '../connectionConfig'
 import { compareVersions } from 'compare-versions'
-import { NotFoundError, SandboxError, TemplateError } from '../errors'
+import { NotFoundError, TemplateError } from '../errors'
 
 /**
  * Options for request to the Sandbox API.
@@ -80,10 +80,18 @@ export class SandboxApi {
     return true
   }
 
+  /**
+   * Pause the sandbox specified by sandbox ID.
+   *
+   * @param sandboxId sandbox ID.
+   * @param opts connection options.
+   *
+   * @returns `true` if the sandbox got paused, `false` if the sandbox was already paused.
+   */
   static async pause(
     sandboxId: string,
     opts?: SandboxApiOpts
-  ): Promise<string> {
+  ): Promise<boolean> {
     const config = new ConnectionConfig(opts)
     const client = new ApiClient(config)
 
@@ -102,7 +110,7 @@ export class SandboxApi {
 
     if (res.error?.code === 409) {
       // Sandbox is already paused
-      throw new SandboxError(`Sandbox ${sandboxId} is already paused`)
+      return false
     }
 
     const err = handleApiError(res)
@@ -110,7 +118,7 @@ export class SandboxApi {
       throw err
     }
 
-    return sandboxId
+    return true
   }
 
   /**
@@ -189,7 +197,7 @@ export class SandboxApi {
     sandboxId: string,
     timeoutMs: number,
     opts?: SandboxApiOpts
-  ): Promise<string> {
+  ): Promise<boolean> {
     const config = new ConnectionConfig(opts)
     const client = new ApiClient(config)
 
@@ -202,6 +210,7 @@ export class SandboxApi {
       body: {
         timeout: this.timeoutToSeconds(timeoutMs),
       },
+      signal: config.getSignal(opts?.requestTimeoutMs),
     })
 
     if (res.error?.code === 404) {
@@ -210,7 +219,7 @@ export class SandboxApi {
 
     if (res.error?.code === 409) {
       // Sandbox is not paused
-      throw new SandboxError(`Sandbox ${sandboxId} is not paused`)
+      return false
     }
 
     const err = handleApiError(res)
@@ -218,10 +227,7 @@ export class SandboxApi {
       throw err
     }
 
-    return this.getSandboxId({
-      sandboxId: res.data!.sandboxID,
-      clientId: res.data!.clientID,
-    })
+    return true
   }
 
   protected static async createSandbox(