Fix error info loss in rest.request() commands and batch-publish

.claude/skills/ably-new-command/references/patterns.md

@@ -661,7 +661,7 @@ Commands must behave strictly according to their documented purpose — no unint
 **Subscribe commands** — passive observers:
 - **Only** listen for new events — must NOT fetch initial state (use `get-all` for that)
 - **NOT enter presence/space** — use `enterSpace: false`. The Spaces SDK's `subscribe()` methods do NOT require `space.enter()`
-- Use the message pattern: `formatProgress("Subscribing to X")` → `formatListening("Listening for X.")`
+- Use the message pattern: `formatProgress("Subscribing to X")` → `formatSuccess("Subscribed to X.")` → `formatListening("Listening for X.")`
 
 **Get-all / get commands** — one-shot queries:
 - **NOT enter presence/space** — `getAll()`, `get()` do NOT require `space.enter()`
@@ -677,6 +677,13 @@ Commands must behave strictly according to their documented purpose — no unint
 - Call `this.markAsEntered()` after every `space.enter()` (enables cleanup)
 - For hold commands, always use manual entry (`enterSpace: false` + `space.enter()` + `markAsEntered()`) for consistency
 
+**Room success message timing** (`setupRoomStatusHandler` in `ChatBaseCommand`):
+- `setupRoomStatusHandler` can emit `successMessage` and `listeningMessage` on `RoomStatus.Attached`, but this fires when `room.attach()` completes — **before** any subscribe/action call that happens after attach.
+- Chat SDK `subscribe()` calls just register a listener (synchronous, no I/O) — they work before or after `room.attach()`. The ordering doesn't affect correctness. But the success message must not claim "Subscribed" before the subscribe call has been made.
+- **Pattern 1** (handler messages): Only use `successMessage`/`listeningMessage` in `setupRoomStatusHandler` when the subscribe call is placed **before** `room.attach()` in the code. Examples: `messages/subscribe`, `typing/subscribe`.
+- **Pattern 2** (manual messages): When the subscribe or action is placed **after** `room.attach()`, print `formatSuccess()` and `formatListening()` manually after the subscribe/action call. Examples: `presence/subscribe`, `occupancy/subscribe`, `reactions/subscribe`, `presence/enter`, `typing/keystroke`.
+- Never say "Connected to room" — the attach event is not a connection event. Use "Subscribed to X in room" for subscribe commands, or an action-specific message ("Entered presence in room", "Started typing in room") for action commands.
+
 ```typescript
 // WRONG — subscribe enters the space
 await this.initializeSpace(flags, spaceName, { enterSpace: true });

AGENTS.md

@@ -251,6 +251,7 @@ Each command type has strict rules about what side effects it may have. Remove u
 - **Set / enter / acquire** = hold state until Ctrl+C / `--duration` (enter, operate, hold — no subscribing after)
 - Call `space.enter()` only when SDK requires it; always call `this.markAsEntered()` after
 - Hold commands use manual entry (`enterSpace: false` + `space.enter()` + `markAsEntered()`) for consistency
+- **Room success messages**: Only use `successMessage` in `setupRoomStatusHandler` when the subscribe call is **before** `room.attach()`. Otherwise, print success/listening manually **after** the subscribe/action. Never say "Connected to room" — use action-specific wording.
 
 See `references/patterns.md` "Command behavior semantics" in the `ably-new-command` skill for full rules, side-effect table, and code examples.
 

src/base-command.ts

@@ -115,6 +115,36 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
   // Core global flags available to all commands (verbose, json, pretty-json, web-cli-help)
   static globalFlags = { ...coreGlobalFlags };
 
+  /**
+   * Strip internal SDK channel suffixes from error messages so users see
+   * room/space names, not underlying channel IDs.
+   * Covers Chat (::$chat::$chatMessages, ::$chat::$typingindicators, etc.),
+   * Spaces (::$space), and Cursors (::$cursors).
+   * Also rewrites "Channel"/"channelId" terminology to "Room"/"Space" when the
+   * suffix reveals the product context.
+   */
+  private static sanitizeSdkErrorMessage(message: string): string {
+    const isRoom = /::\$chat(?:::\$?\w+)*/.test(message);
+    const isSpace = /::\$(?:space|cursors)(?:::\$?\w+)*/.test(message);
+
+    let sanitized = message.replaceAll(
+      /::\$(?:chat|space|cursors)(?:::\$?\w+)*/g,
+      "",
+    );
+
+    if (isRoom) {
+      sanitized = sanitized
+        .replace(/\bChannel denied access\b/, "Room denied access")
+        .replace(/\bchannelId\b/, "room");
+    } else if (isSpace) {
+      sanitized = sanitized
+        .replace(/\bChannel denied access\b/, "Space denied access")
+        .replace(/\bchannelId\b/, "space");
+    }
+
+    return sanitized;
+  }
+
   protected configManager: ConfigManager;
   protected interactiveHelper: InteractiveHelper;
 
@@ -955,20 +985,9 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
             message,
             logData,
           );
-        } else if (level <= 1) {
-          // Standard JSON: Log only SDK ERRORS (level <= 1) to stderr as JSON
-          const errorData = {
-            level,
-            logType: "sdkError",
-            message,
-            timestamp: new Date().toISOString(),
-          };
-          // Log to stderr with standard JSON envelope for consistency
-          this.logToStderr(
-            this.formatJsonRecord(JsonRecordType.Log, errorData, flags),
-          );
         }
-        // If not verbose JSON and level > 1, suppress non-error SDK logs
+        // SDK errors (level <= 1) are surfaced by command-level error handling.
+        // Raw SDK error output is only shown with --verbose (above).
       } else {
         // Non-JSON Mode Handling
         if (flags.verbose && level <= 2) {
@@ -982,12 +1001,10 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
             message,
             logData,
           );
-        } else if (level <= 1) {
-          // SDK errors are handled by setupChannelStateLogging() and fail()
-          // Only show raw SDK errors in verbose mode (handled above)
-          // In non-verbose mode, log to stderr for debugging without polluting stdout
-          this.logToStderr(`${chalk.red.bold(`[AblySDK Error]`)} ${message}`);
         }
+        // SDK errors (level <= 1) are surfaced by setupConnectionStateLogging(),
+        // setupChannelStateLogging(), setupRoomStatusHandler(), and command-level
+        // catch blocks. Raw SDK error output is only shown with --verbose (above).
         // If not verbose non-JSON and level > 1, suppress non-error SDK logs
       }
     };
@@ -1564,6 +1581,9 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
     if (error instanceof Error && "oclif" in error) throw error;
 
     const cmdError = CommandError.from(error, context);
+    cmdError.message = AblyBaseCommand.sanitizeSdkErrorMessage(
+      cmdError.message,
+    );
 
     this.logCliEvent(
       flags,

src/chat-base-command.ts

@@ -113,13 +113,18 @@ export abstract class ChatBaseCommand extends AblyBaseCommand {
       successMessage?: string;
       listeningMessage?: string;
     },
-  ): void {
+  ): { failurePromise: Promise<never> } {
+    let rejectOnFailed!: (error: Error) => void;
+    const failurePromise = new Promise<never>((_, reject) => {
+      rejectOnFailed = reject;
+    });
+
     room.onStatusChange((statusChange) => {
-      let reason: Error | null | string | undefined;
+      let reason: Error | undefined;
       if (statusChange.current === RoomStatus.Failed) {
-        reason = room.error;
+        reason = room.error ?? undefined;
       }
-      const reasonMsg = reason instanceof Error ? reason.message : reason;
+      const reasonMsg = reason?.message;
       this.logCliEvent(
         flags,
         "room",
@@ -146,14 +151,15 @@ export abstract class ChatBaseCommand extends AblyBaseCommand {
           break;
         }
         case RoomStatus.Failed: {
-          this.fail(
-            `Failed to attach to room ${options.roomName}: ${reasonMsg || "Unknown error"}`,
-            flags as BaseFlags,
-            "room",
+          rejectOnFailed(
+            reason || new Error(`Room ${options.roomName} failed`),
           );
+          break;
         }
         // No default
       }
     });
+
+    return { failurePromise };
   }
 }

src/commands/channels/batch-publish.ts

@@ -2,6 +2,7 @@ import { Args, Flags } from "@oclif/core";
 import chalk from "chalk";
 
 import { AblyBaseCommand } from "../../base-command.js";
+import { CommandError } from "../../errors/command-error.js";
 import { productApiFlags } from "../../flags.js";
 import {
   formatProgress,
@@ -255,7 +256,7 @@ export default class ChannelsBatchPublish extends AblyBaseCommand {
             // This is a partial success with batchResponse field
             if (!this.shouldSuppressOutput(flags)) {
               if (this.shouldOutputJson(flags)) {
-                this.fail(errorInfo.error.message, flags, "batchPublish", {
+                this.fail(errorInfo.error, flags, "batchPublish", {
                   channels: Array.isArray(batchContentObj.channels)
                     ? batchContentObj.channels
                     : [batchContentObj.channels],
@@ -286,44 +287,36 @@ export default class ChannelsBatchPublish extends AblyBaseCommand {
             }
           } else {
             // Complete failure
-            const errMsg = errorInfo.error
-              ? errorInfo.error.message
-              : "Unknown error";
-            const errorCode = errorInfo.error
-              ? errorInfo.error.code
-              : response.statusCode;
             this.fail(
-              `Batch publish failed: ${errMsg} (${errorCode})`,
+              errorInfo.error ||
+                CommandError.fromHttpResponse(response, "Batch publish failed"),
               flags,
               "batchPublish",
             );
           }
         } else {
           this.fail(
-            `Batch publish failed with status code ${response.statusCode}`,
+            CommandError.fromHttpResponse(response, "Batch publish failed"),
             flags,
             "batchPublish",
           );
         }
       } else {
         // Other error response
         const responseData = response.items;
-        let errMsg = "Unknown error";
-        let errorCode = response.statusCode;
+        let errorSource: unknown = CommandError.fromHttpResponse(
+          response,
+          "Batch publish failed",
+        );
 
         if (typeof responseData === "object" && !Array.isArray(responseData)) {
           const errorInfo = responseData as ErrorInfo;
           if (errorInfo.error) {
-            errMsg = errorInfo.error.message || errMsg;
-            errorCode = errorInfo.error.code || errorCode;
+            errorSource = errorInfo.error;
           }
         }
 
-        this.fail(
-          `Batch publish failed: ${errMsg} (${errorCode})`,
-          flags,
-          "batchPublish",
-        );
+        this.fail(errorSource, flags, "batchPublish");
       }
     } catch (error) {
       this.fail(error, flags, "batchPublish");

src/commands/channels/list.ts

@@ -1,5 +1,6 @@
 import { Flags } from "@oclif/core";
 import { AblyBaseCommand } from "../../base-command.js";
+import { CommandError } from "../../errors/command-error.js";
 import { productApiFlags } from "../../flags.js";
 import {
   formatCountLabel,
@@ -78,7 +79,10 @@ export default class ChannelsList extends AblyBaseCommand {
 
       if (channelsResponse.statusCode !== 200) {
         this.fail(
-          `Failed to list channels: ${channelsResponse.statusCode}`,
+          CommandError.fromHttpResponse(
+            channelsResponse,
+            "Failed to list channels",
+          ),
           flags,
           "channelList",
         );

src/commands/channels/occupancy/get.ts

@@ -1,6 +1,7 @@
 import { Args } from "@oclif/core";
 
 import { AblyBaseCommand } from "../../../base-command.js";
+import { CommandError } from "../../../errors/command-error.js";
 import { productApiFlags } from "../../../flags.js";
 import { formatLabel, formatResource } from "../../../utils/output.js";
 
@@ -57,6 +58,18 @@ export default class ChannelsOccupancyGet extends AblyBaseCommand {
         null, // body
       );
 
+      if (channelDetails.statusCode !== 200) {
+        this.fail(
+          CommandError.fromHttpResponse(
+            channelDetails,
+            "Failed to get channel occupancy",
+          ),
+          flags,
+          "occupancyGet",
+          { channel: channelName },
+        );
+      }
+
       const occupancyData = ((channelDetails as { items?: unknown[] })
         .items?.[0] ?? {}) as Record<string, unknown>;
       const statusObj = occupancyData.status as

src/commands/push/batch-publish.ts

@@ -3,6 +3,7 @@ import * as fs from "node:fs";
 import * as path from "node:path";
 
 import { AblyBaseCommand } from "../../base-command.js";
+import { CommandError } from "../../errors/command-error.js";
 import { productApiFlags } from "../../flags.js";
 import { promptForConfirmation } from "../../utils/prompt-confirmation.js";
 import { BaseFlags } from "../../types/cli.js";
@@ -252,6 +253,17 @@ export default class PushBatchPublish extends AblyBaseCommand {
           recipientItems.map(({ entry }) => entry),
         );
 
+        if (response.statusCode < 200 || response.statusCode >= 300) {
+          this.fail(
+            CommandError.fromHttpResponse(
+              response,
+              "Batch push publish failed",
+            ),
+            flags as BaseFlags,
+            "pushBatchPublish",
+          );
+        }
+
         const items = response.items as Record<string, unknown>[];
         const failedWithIndex = items
           .map((item, i) => ({
@@ -326,6 +338,17 @@ export default class PushBatchPublish extends AblyBaseCommand {
           channelBatchSpecs,
         );
 
+        if (response.statusCode < 200 || response.statusCode >= 300) {
+          this.fail(
+            CommandError.fromHttpResponse(
+              response,
+              "Batch channel publish failed",
+            ),
+            flags as BaseFlags,
+            "pushBatchPublish",
+          );
+        }
+
         const responseItems = response.items as BatchResponseItem[];
         const failedWithIndex = responseItems
           .map((item, i) => ({

src/commands/rooms/list.ts

@@ -1,5 +1,6 @@
 import { Flags } from "@oclif/core";
 import { ChatBaseCommand } from "../../chat-base-command.js";
+import { CommandError } from "../../errors/command-error.js";
 import { productApiFlags } from "../../flags.js";
 import {
   formatCountLabel,
@@ -79,7 +80,10 @@ export default class RoomsList extends ChatBaseCommand {
 
       if (channelsResponse.statusCode !== 200) {
         this.fail(
-          `Failed to list rooms: ${channelsResponse.statusCode}`,
+          CommandError.fromHttpResponse(
+            channelsResponse,
+            "Failed to list rooms",
+          ),
           flags,
           "roomList",
         );

src/commands/rooms/messages/reactions/subscribe.ts

@@ -14,12 +14,14 @@ import {
   productApiFlags,
 } from "../../../../flags.js";
 import {
-  formatProgress,
-  formatResource,
-  formatTimestamp,
   formatClientId,
   formatEventType,
   formatLabel,
+  formatListening,
+  formatProgress,
+  formatResource,
+  formatSuccess,
+  formatTimestamp,
 } from "../../../../utils/output.js";
 
 export default class MessagesReactionsSubscribe extends ChatBaseCommand {
@@ -110,10 +112,8 @@ export default class MessagesReactionsSubscribe extends ChatBaseCommand {
       this.logCliEvent(flags, "room", "gotRoom", `Got room handle for ${room}`);
 
       // Subscribe to room status changes
-      this.setupRoomStatusHandler(chatRoom, flags, {
+      const { failurePromise } = this.setupRoomStatusHandler(chatRoom, flags, {
         roomName: room,
-        successMessage: "Connected to Ably.",
-        listeningMessage: `Listening for message reactions in room ${formatResource(room)}.`,
       });
 
       // Attach to the room
@@ -233,6 +233,15 @@ export default class MessagesReactionsSubscribe extends ChatBaseCommand {
         );
       }
 
+      if (!this.shouldOutputJson(flags)) {
+        this.log(
+          formatSuccess(
+            `Subscribed to message reactions in room: ${formatResource(room)}.`,
+          ),
+        );
+        this.log(formatListening("Listening for message reactions."));
+      }
+
       this.logCliEvent(
         flags,
         "reactions",
@@ -241,7 +250,10 @@ export default class MessagesReactionsSubscribe extends ChatBaseCommand {
       );
 
       // Wait until the user interrupts or the optional duration elapses
-      await this.waitAndTrackCleanup(flags, "reactions", flags.duration);
+      await Promise.race([
+        this.waitAndTrackCleanup(flags, "reactions", flags.duration),
+        failurePromise,
+      ]);
     } catch (error) {
       this.fail(error, flags, "roomMessageReactionSubscribe", {
         room: args.room,