diff --git a/.claude/skills/ably-new-command/references/patterns.md b/.claude/skills/ably-new-command/references/patterns.md index 778309f3..ecae904f 100644 --- a/.claude/skills/ably-new-command/references/patterns.md +++ b/.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 }); diff --git a/AGENTS.md b/AGENTS.md index 79795a07..573f8d38 100644 --- a/AGENTS.md +++ b/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. diff --git a/src/base-command.ts b/src/base-command.ts index 4d27d4ee..aceb0618 100644 --- a/src/base-command.ts +++ b/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, diff --git a/src/chat-base-command.ts b/src/chat-base-command.ts index f0fcc0b6..46b063a4 100644 --- a/src/chat-base-command.ts +++ b/src/chat-base-command.ts @@ -113,13 +113,18 @@ export abstract class ChatBaseCommand extends AblyBaseCommand { successMessage?: string; listeningMessage?: string; }, - ): void { + ): { failurePromise: Promise } { + let rejectOnFailed!: (error: Error) => void; + const failurePromise = new Promise((_, 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 }; } } diff --git a/src/commands/channels/batch-publish.ts b/src/commands/channels/batch-publish.ts index f4c2b06d..37ba54e6 100644 --- a/src/commands/channels/batch-publish.ts +++ b/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,21 +287,16 @@ 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", ); @@ -308,22 +304,19 @@ export default class ChannelsBatchPublish extends AblyBaseCommand { } 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"); diff --git a/src/commands/channels/list.ts b/src/commands/channels/list.ts index f978f31a..eca0fdbf 100644 --- a/src/commands/channels/list.ts +++ b/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", ); diff --git a/src/commands/channels/occupancy/get.ts b/src/commands/channels/occupancy/get.ts index 4322b910..cfb35893 100644 --- a/src/commands/channels/occupancy/get.ts +++ b/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; const statusObj = occupancyData.status as diff --git a/src/commands/push/batch-publish.ts b/src/commands/push/batch-publish.ts index 35681796..d8e96de6 100644 --- a/src/commands/push/batch-publish.ts +++ b/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[]; 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) => ({ diff --git a/src/commands/rooms/list.ts b/src/commands/rooms/list.ts index a8cc36bc..b855920b 100644 --- a/src/commands/rooms/list.ts +++ b/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", ); diff --git a/src/commands/rooms/messages/reactions/subscribe.ts b/src/commands/rooms/messages/reactions/subscribe.ts index 2f08cc21..4583fd17 100644 --- a/src/commands/rooms/messages/reactions/subscribe.ts +++ b/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, diff --git a/src/commands/rooms/messages/subscribe.ts b/src/commands/rooms/messages/subscribe.ts index 6d399dc8..6c2f27ab 100644 --- a/src/commands/rooms/messages/subscribe.ts +++ b/src/commands/rooms/messages/subscribe.ts @@ -71,7 +71,7 @@ export default class MessagesSubscribe extends ChatBaseCommand { private async subscribeToRoom( roomName: string, flags: Record, - ): Promise { + ): Promise<{ failurePromise: Promise }> { // Get the room this.logCliEvent( flags, @@ -167,7 +167,7 @@ export default class MessagesSubscribe extends ChatBaseCommand { `Subscribed to messages in room ${roomName}`, ); - this.setupRoomStatusHandler(room, flags, { + const { failurePromise } = this.setupRoomStatusHandler(room, flags, { roomName, successMessage: `Subscribed to room: ${formatResource(roomName)}.`, listeningMessage: "Listening for messages.", @@ -187,6 +187,8 @@ export default class MessagesSubscribe extends ChatBaseCommand { "attachCallComplete", `room.attach() call complete for ${roomName}. Waiting for status change to 'attached'.`, ); + + return { failurePromise }; } async run(): Promise { @@ -252,8 +254,10 @@ export default class MessagesSubscribe extends ChatBaseCommand { }); // Subscribe to all rooms + const failurePromises: Promise[] = []; for (const roomName of this.roomNames) { - await this.subscribeToRoom(roomName, flags); + const { failurePromise } = await this.subscribeToRoom(roomName, flags); + failurePromises.push(failurePromise); } this.logCliEvent( @@ -264,7 +268,10 @@ export default class MessagesSubscribe extends ChatBaseCommand { ); // Wait until the user interrupts or the optional duration elapses - await this.waitAndTrackCleanup(flags, "subscribe", flags.duration); + await Promise.race([ + this.waitAndTrackCleanup(flags, "subscribe", flags.duration), + ...failurePromises, + ]); } catch (error) { this.fail(error, flags, "roomMessageSubscribe", { rooms: this.roomNames, diff --git a/src/commands/rooms/occupancy/subscribe.ts b/src/commands/rooms/occupancy/subscribe.ts index 840195ce..8f16d797 100644 --- a/src/commands/rooms/occupancy/subscribe.ts +++ b/src/commands/rooms/occupancy/subscribe.ts @@ -5,8 +5,10 @@ import { ChatBaseCommand } from "../../../chat-base-command.js"; import { clientIdFlag, durationFlag, productApiFlags } from "../../../flags.js"; import { formatLabel, + formatListening, formatProgress, formatResource, + formatSuccess, formatTimestamp, } from "../../../utils/output.js"; @@ -92,10 +94,8 @@ export default class RoomsOccupancySubscribe extends ChatBaseCommand { ); // Subscribe to room status changes - this.setupRoomStatusHandler(room, flags, { + const { failurePromise } = this.setupRoomStatusHandler(room, flags, { roomName: this.roomName, - successMessage: `Subscribed to occupancy in room: ${formatResource(this.roomName)}.`, - listeningMessage: "Listening for occupancy updates.", }); // Attach to the room @@ -135,8 +135,20 @@ export default class RoomsOccupancySubscribe extends ChatBaseCommand { "Subscribed to occupancy updates", ); + if (!this.shouldOutputJson(flags)) { + this.log( + formatSuccess( + `Subscribed to occupancy in room: ${formatResource(this.roomName)}.`, + ), + ); + this.log(formatListening("Listening for occupancy updates.")); + } + // Wait until the user interrupts or the optional duration elapses - await this.waitAndTrackCleanup(flags, "occupancy", flags.duration); + await Promise.race([ + this.waitAndTrackCleanup(flags, "occupancy", flags.duration), + failurePromise, + ]); } catch (error) { this.fail(error, flags, "roomOccupancySubscribe", { room: this.roomName, diff --git a/src/commands/rooms/presence/enter.ts b/src/commands/rooms/presence/enter.ts index efcb2152..c67ee444 100644 --- a/src/commands/rooms/presence/enter.ts +++ b/src/commands/rooms/presence/enter.ts @@ -91,12 +91,13 @@ export default class RoomsPresenceEnter extends ChatBaseCommand { this.room = await this.chatClient.rooms.get(this.roomName); const currentRoom = this.room; + let failurePromise: Promise | undefined; if (flags["show-others"]) { // Subscribe to room status changes only when showing others - this.setupRoomStatusHandler(currentRoom, flags, { + ({ failurePromise } = this.setupRoomStatusHandler(currentRoom, flags, { roomName: this.roomName, - successMessage: `Connected to room: ${formatResource(this.roomName)}.`, - }); + // Success message printed after presence.enter() completes + })); currentRoom.presence.subscribe((event: PresenceEvent) => { const member = event.member; @@ -214,7 +215,11 @@ export default class RoomsPresenceEnter extends ChatBaseCommand { ); // Wait until the user interrupts or the optional duration elapses - await this.waitAndTrackCleanup(flags, "presence", flags.duration); + const waitPromises: Promise[] = [ + this.waitAndTrackCleanup(flags, "presence", flags.duration), + ]; + if (failurePromise) waitPromises.push(failurePromise); + await Promise.race(waitPromises); } catch (error) { this.fail(error, flags, "roomPresenceEnter", { room: this.roomName, diff --git a/src/commands/rooms/presence/subscribe.ts b/src/commands/rooms/presence/subscribe.ts index d7f5b6f5..7fe8cb1f 100644 --- a/src/commands/rooms/presence/subscribe.ts +++ b/src/commands/rooms/presence/subscribe.ts @@ -73,11 +73,14 @@ export default class RoomsPresenceSubscribe extends ChatBaseCommand { this.room = await this.chatClient.rooms.get(this.roomName); const currentRoom = this.room; - this.setupRoomStatusHandler(currentRoom, flags, { - roomName: this.roomName, - successMessage: `Connected to room: ${formatResource(this.roomName)}.`, - listeningMessage: undefined, - }); + const { failurePromise } = this.setupRoomStatusHandler( + currentRoom, + flags, + { + roomName: this.roomName, + listeningMessage: undefined, + }, + ); await currentRoom.attach(); @@ -161,7 +164,10 @@ export default class RoomsPresenceSubscribe extends ChatBaseCommand { } // Wait until the user interrupts or the optional duration elapses - await this.waitAndTrackCleanup(flags, "presence", flags.duration); + await Promise.race([ + this.waitAndTrackCleanup(flags, "presence", flags.duration), + failurePromise, + ]); } catch (error) { this.fail(error, flags, "roomPresenceSubscribe", { room: this.roomName, diff --git a/src/commands/rooms/reactions/send.ts b/src/commands/rooms/reactions/send.ts index dd123183..55192bad 100644 --- a/src/commands/rooms/reactions/send.ts +++ b/src/commands/rooms/reactions/send.ts @@ -96,7 +96,10 @@ export default class RoomsReactionsSend extends ChatBaseCommand { ); // Subscribe to room status changes - this.setupRoomStatusHandler(room, flags, { roomName }); + const { failurePromise } = this.setupRoomStatusHandler(room, flags, { + roomName, + }); + failurePromise.catch(() => {}); // one-shot command; attach() handles failure // Attach to the room this.logCliEvent( diff --git a/src/commands/rooms/reactions/subscribe.ts b/src/commands/rooms/reactions/subscribe.ts index e8b5c0d6..ad753deb 100644 --- a/src/commands/rooms/reactions/subscribe.ts +++ b/src/commands/rooms/reactions/subscribe.ts @@ -6,10 +6,12 @@ import { ChatBaseCommand } from "../../../chat-base-command.js"; import { clientIdFlag, durationFlag, productApiFlags } from "../../../flags.js"; import { formatClientId, + formatLabel, + formatListening, formatProgress, formatResource, + formatSuccess, formatTimestamp, - formatLabel, } from "../../../utils/output.js"; export default class RoomsReactionsSubscribe extends ChatBaseCommand { @@ -88,10 +90,8 @@ export default class RoomsReactionsSubscribe extends ChatBaseCommand { ); // Subscribe to room status changes - this.setupRoomStatusHandler(room, flags, { + const { failurePromise } = this.setupRoomStatusHandler(room, flags, { roomName, - successMessage: `Subscribed to reactions in room: ${formatResource(roomName)}.`, - listeningMessage: "Listening for reactions.", }); // Attach to the room @@ -154,6 +154,15 @@ export default class RoomsReactionsSubscribe extends ChatBaseCommand { "Subscribed to reactions", ); + if (!this.shouldOutputJson(flags)) { + this.log( + formatSuccess( + `Subscribed to reactions in room: ${formatResource(roomName)}.`, + ), + ); + this.log(formatListening("Listening for reactions.")); + } + this.logCliEvent( flags, "reactions", @@ -162,7 +171,10 @@ export default class RoomsReactionsSubscribe 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, "roomReactionSubscribe", { room: args.room }); } diff --git a/src/commands/rooms/typing/keystroke.ts b/src/commands/rooms/typing/keystroke.ts index ed31d998..197c3353 100644 --- a/src/commands/rooms/typing/keystroke.ts +++ b/src/commands/rooms/typing/keystroke.ts @@ -1,4 +1,4 @@ -import { RoomStatus, ChatClient } from "@ably/chat"; +import { ChatClient } from "@ably/chat"; import { Args, Flags } from "@oclif/core"; import { ChatBaseCommand } from "../../../chat-base-command.js"; @@ -96,101 +96,9 @@ export default class TypingKeystroke extends ChatBaseCommand { `Got room handle for ${roomName}`, ); - // Subscribe to room status changes — combines logging and keystroke trigger - room.onStatusChange((statusChange) => { - let reason: Error | null | string | undefined; - if (statusChange.current === RoomStatus.Failed) { - reason = room.error; - } - const reasonMsg = reason instanceof Error ? reason.message : reason; - this.logCliEvent( - flags, - "room", - `status-${statusChange.current}`, - `Room status changed to ${statusChange.current}`, - { reason: reasonMsg, room: roomName }, - ); - - if (statusChange.current === RoomStatus.Attached) { - if (!this.shouldOutputJson(flags)) { - this.log( - formatSuccess(`Connected to room: ${formatResource(roomName)}.`), - ); - } - - // Start typing immediately - this.logCliEvent( - flags, - "typing", - "startAttempt", - "Attempting to start typing...", - ); - room.typing - .keystroke() - .then(() => { - this.logCliEvent(flags, "typing", "started", "Started typing"); - if (this.shouldOutputJson(flags)) { - this.logJsonResult( - { - typing: { - room: roomName, - isTyping: true, - autoType: Boolean(flags["auto-type"]), - }, - }, - flags, - ); - } else { - this.log( - formatSuccess( - `Started typing in room: ${formatResource(roomName)}.`, - ), - ); - if (flags["auto-type"]) { - this.log( - formatListening( - "Will automatically remain typing until terminated.", - ), - ); - } else { - this.log( - formatListening( - "Sent a single typing indicator. Use --auto-type to keep typing automatically.", - ), - ); - } - } - - // Keep typing active by calling keystroke() periodically if autoType is enabled - if (this.typingIntervalId) clearInterval(this.typingIntervalId); - - if (flags["auto-type"]) { - this.typingIntervalId = setInterval(() => { - room.typing.keystroke().catch((error: Error) => { - this.logCliEvent( - flags, - "typing", - "startErrorPeriodic", - `Error refreshing typing state: ${error.message}`, - { error: error.message }, - ); - }); - }, KEYSTROKE_INTERVAL); - } - }) - .catch((error: Error) => { - this.fail(error, flags, "roomTypingKeystroke", { - room: roomName, - }); - }); - } else if (statusChange.current === RoomStatus.Failed) { - this.fail( - `Failed to attach to room ${roomName}: ${reasonMsg || "Unknown error"}`, - flags, - "roomTypingKeystroke", - { room: roomName }, - ); - } + // Subscribe to room status changes + const { failurePromise } = this.setupRoomStatusHandler(room, flags, { + roomName, }); // Attach to the room @@ -201,7 +109,61 @@ export default class TypingKeystroke extends ChatBaseCommand { `Attaching to room ${roomName}`, ); await room.attach(); - // Successful attach and initial typing start logged by onStatusChange handler + + // Start typing + this.logCliEvent( + flags, + "typing", + "startAttempt", + "Attempting to start typing...", + ); + await room.typing.keystroke(); + this.logCliEvent(flags, "typing", "started", "Started typing"); + + if (this.shouldOutputJson(flags)) { + this.logJsonResult( + { + typing: { + room: roomName, + isTyping: true, + autoType: Boolean(flags["auto-type"]), + }, + }, + flags, + ); + } else { + this.log( + formatSuccess(`Started typing in room: ${formatResource(roomName)}.`), + ); + if (flags["auto-type"]) { + this.log( + formatListening( + "Will automatically remain typing until terminated.", + ), + ); + } else { + this.log( + formatListening( + "Sent a single typing indicator. Use --auto-type to keep typing automatically.", + ), + ); + } + } + + // Keep typing active by calling keystroke() periodically if autoType is enabled + if (flags["auto-type"]) { + this.typingIntervalId = setInterval(() => { + room.typing.keystroke().catch((error: Error) => { + this.logCliEvent( + flags, + "typing", + "startErrorPeriodic", + `Error refreshing typing state: ${error.message}`, + { error: error.message }, + ); + }); + }, KEYSTROKE_INTERVAL); + } this.logCliEvent( flags, @@ -210,8 +172,11 @@ export default class TypingKeystroke extends ChatBaseCommand { "Maintaining typing status...", ); - // Decide how long to remain connected - await this.waitAndTrackCleanup(flags, "typing", flags.duration); + // Wait until the user interrupts, duration elapses, or the room fails + await Promise.race([ + this.waitAndTrackCleanup(flags, "typing", flags.duration), + failurePromise, + ]); } catch (error) { this.fail(error, flags, "roomTypingKeystroke", { room: args.room }); } diff --git a/src/commands/rooms/typing/subscribe.ts b/src/commands/rooms/typing/subscribe.ts index b811bd1b..b53ae840 100644 --- a/src/commands/rooms/typing/subscribe.ts +++ b/src/commands/rooms/typing/subscribe.ts @@ -69,7 +69,7 @@ export default class TypingSubscribe extends ChatBaseCommand { ); // Subscribe to room status changes - this.setupRoomStatusHandler(room, flags, { + const { failurePromise } = this.setupRoomStatusHandler(room, flags, { roomName, successMessage: `Subscribed to typing in room: ${formatResource(roomName)}.`, listeningMessage: "Listening for typing indicators.", @@ -157,7 +157,10 @@ export default class TypingSubscribe extends ChatBaseCommand { ); // Wait until the user interrupts or the optional duration elapses - await this.waitAndTrackCleanup(flags, "typing", flags.duration); + await Promise.race([ + this.waitAndTrackCleanup(flags, "typing", flags.duration), + failurePromise, + ]); } catch (error) { this.fail(error, flags, "roomTypingSubscribe", { room: args.room }); } diff --git a/src/commands/spaces/get.ts b/src/commands/spaces/get.ts index 9e40cae3..86fb67e0 100644 --- a/src/commands/spaces/get.ts +++ b/src/commands/spaces/get.ts @@ -1,5 +1,6 @@ import { Args } from "@oclif/core"; +import { CommandError } from "../../errors/command-error.js"; import { productApiFlags } from "../../flags.js"; import { SpacesBaseCommand } from "../../spaces-base-command.js"; import { @@ -90,7 +91,7 @@ export default class SpacesGet extends SpacesBaseCommand { if (response.statusCode !== 200) { this.fail( - `Failed to fetch space: ${response.statusCode}`, + CommandError.fromHttpResponse(response, "Failed to fetch space"), flags, "spaceGet", ); diff --git a/src/commands/spaces/list.ts b/src/commands/spaces/list.ts index f1e21c2b..74870f28 100644 --- a/src/commands/spaces/list.ts +++ b/src/commands/spaces/list.ts @@ -1,5 +1,6 @@ import { Flags } from "@oclif/core"; +import { CommandError } from "../../errors/command-error.js"; import { productApiFlags } from "../../flags.js"; import { formatCountLabel, @@ -73,7 +74,10 @@ export default class SpacesList extends SpacesBaseCommand { if (channelsResponse.statusCode !== 200) { this.fail( - `Failed to list spaces: ${channelsResponse.statusCode}`, + CommandError.fromHttpResponse( + channelsResponse, + "Failed to list spaces", + ), flags, "spaceList", ); diff --git a/src/commands/spaces/occupancy/get.ts b/src/commands/spaces/occupancy/get.ts index 56b9716d..be59bca0 100644 --- a/src/commands/spaces/occupancy/get.ts +++ b/src/commands/spaces/occupancy/get.ts @@ -1,6 +1,7 @@ import { Args } from "@oclif/core"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; +import { CommandError } from "../../../errors/command-error.js"; import { productApiFlags } from "../../../flags.js"; import { formatLabel, formatResource } from "../../../utils/output.js"; @@ -49,6 +50,18 @@ export default class SpacesOccupancyGet extends SpacesBaseCommand { null, ); + if (channelDetails.statusCode !== 200) { + this.fail( + CommandError.fromHttpResponse( + channelDetails, + "Failed to get space occupancy", + ), + flags, + "spacesOccupancyGet", + { spaceName }, + ); + } + const occupancyData = (channelDetails.items[0] ?? {}) as Record< string, unknown diff --git a/src/errors/command-error.ts b/src/errors/command-error.ts index 5ba78e59..9d3c45b9 100644 --- a/src/errors/command-error.ts +++ b/src/errors/command-error.ts @@ -86,6 +86,22 @@ export class CommandError extends Error { return new CommandError(error.message, { context, cause: error }); } + // Duck-type plain objects with a message property (e.g., parsed JSON error bodies) + if ( + typeof error === "object" && + error !== null && + "message" in error && + typeof (error as Record).message === "string" + ) { + const obj = error as Record; + return new CommandError(obj.message as string, { + code: typeof obj.code === "number" ? obj.code : undefined, + statusCode: + typeof obj.statusCode === "number" ? obj.statusCode : undefined, + context, + }); + } + if (typeof error === "string") { return new CommandError(error, { context }); } @@ -93,6 +109,26 @@ export class CommandError extends Error { return new CommandError(String(error), { context }); } + /** + * Create a CommandError from an HttpPaginatedResponse error. + * Extracts errorCode, errorMessage, and statusCode from the response. + */ + static fromHttpResponse( + response: { + statusCode: number; + errorCode?: number | null; + errorMessage?: string | null; + }, + action: string, + ): CommandError { + const message = + response.errorMessage || `${action} (status ${response.statusCode})`; + return new CommandError(message, { + code: response.errorCode || undefined, + statusCode: response.statusCode, + }); + } + /** Produce JSON-safe data for the error envelope */ toJsonData(hint?: string): Record { const errorObj: Record = { diff --git a/test/unit/base/base-command-enhanced.test.ts b/test/unit/base/base-command-enhanced.test.ts index 6cea79e4..2c0a9b2b 100644 --- a/test/unit/base/base-command-enhanced.test.ts +++ b/test/unit/base/base-command-enhanced.test.ts @@ -493,5 +493,71 @@ describe("AblyBaseCommand - Enhanced Coverage", function () { // No stray undefined values expect(raw).not.toContain("undefined"); }); + + it("should strip ::$chat suffixes and rewrite to Room terminology", function () { + const msg = + "Channel denied access based on given capability; channelId = abc::$chat::$chatMessages"; + expect(() => command.testFail(new Error(msg), {}, "room")).toThrow( + "Room denied access based on given capability; room = abc", + ); + }); + + it("should strip ::$chat::$typingindicators suffix", function () { + const msg = + "Channel denied access based on given capability; channelId = abc::$chat::$typingindicators"; + expect(() => command.testFail(new Error(msg), {}, "room")).toThrow( + "Room denied access based on given capability; room = abc", + ); + }); + + it("should strip ::$space suffixes and rewrite to Space terminology", function () { + const msg = + "Channel denied access based on given capability; channelId = myspace::$space"; + expect(() => command.testFail(new Error(msg), {}, "space")).toThrow( + "Space denied access based on given capability; space = myspace", + ); + }); + + it("should strip ::$cursors suffixes and rewrite to Space terminology", function () { + const msg = + "Channel denied access based on given capability; channelId = myspace::$cursors"; + expect(() => command.testFail(new Error(msg), {}, "cursors")).toThrow( + "Space denied access based on given capability; space = myspace", + ); + }); + + it("should preserve Ably error codes when sanitizing channel suffixes", function () { + const mockConfig = { root: "" } as unknown as Config; + const cmd = new TestCommand(["--json"], mockConfig); + const ablyError = Object.assign( + new Error( + "Channel denied access based on given capability; channelId = room1::$chat::$typingindicators", + ), + { code: 40160, statusCode: 401 }, + ); + + expect(() => cmd.testFail(ablyError, { json: true }, "room")).toThrow(); + + const parsed = JSON.parse(cmd.capturedOutput[0]); + expect(parsed.error.message).toBe( + "Room denied access based on given capability; room = room1", + ); + expect(parsed.error.code).toBe(40160); + expect(parsed.error.statusCode).toBe(401); + }); + + it("should not modify errors without SDK channel suffixes", function () { + expect(() => + command.testFail(new Error("Connection timeout"), {}, "connection"), + ).toThrow("Connection timeout"); + }); + + it("should not rewrite Channel terminology for plain channel errors", function () { + const msg = + "Channel denied access based on given capability; channelId = my-channel"; + expect(() => command.testFail(new Error(msg), {}, "channel")).toThrow( + "Channel denied access based on given capability; channelId = my-channel", + ); + }); }); }); diff --git a/test/unit/commands/channels/batch-publish.test.ts b/test/unit/commands/channels/batch-publish.test.ts index 9cd205fa..371173a9 100644 --- a/test/unit/commands/channels/batch-publish.test.ts +++ b/test/unit/commands/channels/batch-publish.test.ts @@ -262,6 +262,48 @@ describe("channels:batch-publish command", () => { expect(stdout).toContain("Invalid channel name (40000)"); }); + it("should surface error code and message from complete failure response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + statusCode: 400, + errorCode: 40000, + errorMessage: "Bad request", + items: { + error: { code: 40000, message: "Invalid message format" }, + }, + }); + + const { error } = await runCommand( + ["channels:batch-publish", "--channels", "channel1", '{"data":"test"}'], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid message format"); + expect(error?.message).not.toContain("[object Object]"); + }); + + it("should surface error from non-400 error response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + statusCode: 500, + errorCode: 50000, + errorMessage: "Internal error", + items: { + error: { code: 50000, message: "Server error occurred" }, + }, + }); + + const { error } = await runCommand( + ["channels:batch-publish", "--channels", "channel1", '{"data":"test"}'], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Server error occurred"); + expect(error?.message).not.toContain("[object Object]"); + }); + it("should handle API errors in JSON mode", async () => { const mock = getMockAblyRest(); mock.request.mockRejectedValue(new Error("Network error")); diff --git a/test/unit/commands/channels/list.test.ts b/test/unit/commands/channels/list.test.ts index 26ab6778..30dbf269 100644 --- a/test/unit/commands/channels/list.test.ts +++ b/test/unit/commands/channels/list.test.ts @@ -72,6 +72,21 @@ describe("channels:list command", () => { expect(error?.message).toContain("Failed to list channels"); }); + it("should surface errorCode and errorMessage from HTTP response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + statusCode: 401, + errorCode: 40101, + errorMessage: "Invalid credentials", + }); + + const { error } = await runCommand(["channels:list"], import.meta.url); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid credentials"); + expect(error?.message).toContain("40101"); + }); + it("should respect limit flag", async () => { const mock = getMockAblyRest(); diff --git a/test/unit/commands/channels/occupancy/get.test.ts b/test/unit/commands/channels/occupancy/get.test.ts index bc067898..fd3662a0 100644 --- a/test/unit/commands/channels/occupancy/get.test.ts +++ b/test/unit/commands/channels/occupancy/get.test.ts @@ -12,6 +12,7 @@ describe("ChannelsOccupancyGet", function () { const mock = getMockAblyRest(); // Set up default mock response for REST request mock.request.mockResolvedValue({ + statusCode: 200, items: [ { status: { @@ -95,6 +96,7 @@ describe("ChannelsOccupancyGet", function () { const mock = getMockAblyRest(); // Override mock to return empty metrics mock.request.mockResolvedValue({ + statusCode: 200, occupancy: { metrics: null, }, @@ -152,5 +154,42 @@ describe("ChannelsOccupancyGet", function () { expect(error).toBeDefined(); }); + + it("should handle HTTP error responses from API", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + success: false, + statusCode: 401, + errorCode: 40101, + errorMessage: "Invalid credentials", + }); + + const { error } = await runCommand( + ["channels:occupancy:get", "test-channel"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid credentials"); + }); + + it("should surface errorCode and errorMessage from HTTP response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + success: false, + statusCode: 404, + errorCode: 40400, + errorMessage: "Channel not found", + }); + + const { error } = await runCommand( + ["channels:occupancy:get", "test-channel"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Channel not found"); + expect(error?.message).toContain("40400"); + }); }); }); diff --git a/test/unit/commands/push/batch-publish.test.ts b/test/unit/commands/push/batch-publish.test.ts index a4827059..545de6cd 100644 --- a/test/unit/commands/push/batch-publish.test.ts +++ b/test/unit/commands/push/batch-publish.test.ts @@ -237,5 +237,48 @@ describe("push:batch-publish command", () => { expect(result.publish.failedItems).toHaveLength(1); expect(result.publish.failedItems[0].originalIndex).toBe(1); }); + + it("should handle HTTP error responses from API", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + success: false, + statusCode: 401, + errorCode: 40101, + errorMessage: "Invalid credentials", + }); + + const payload = + '[{"recipient":{"deviceId":"dev-1"},"payload":{"notification":{"title":"Hello"}}}]'; + + const { error } = await runCommand( + ["push:batch-publish", payload, "--force"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid credentials"); + }); + + it("should surface errorCode and errorMessage from HTTP response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + success: false, + statusCode: 403, + errorCode: 40300, + errorMessage: "Push not enabled", + }); + + const payload = + '[{"recipient":{"deviceId":"dev-1"},"payload":{"notification":{"title":"Hello"}}}]'; + + const { error } = await runCommand( + ["push:batch-publish", payload, "--force"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Push not enabled"); + expect(error?.message).toContain("40300"); + }); }); }); diff --git a/test/unit/commands/rooms/list.test.ts b/test/unit/commands/rooms/list.test.ts index 1d4f37fe..a17fcac8 100644 --- a/test/unit/commands/rooms/list.test.ts +++ b/test/unit/commands/rooms/list.test.ts @@ -138,16 +138,6 @@ describe("rooms:list command", () => { expect(json).toHaveProperty("hasMore", false); }); - it("should handle non-200 response with error", async () => { - const mock = getMockAblyRest(); - mock.request.mockResolvedValue({ statusCode: 400, error: "Bad Request" }); - - const { error } = await runCommand(["rooms:list"], import.meta.url); - - expect(error).toBeDefined(); - expect(error?.message).toContain("Failed to list rooms"); - }); - describe("functionality", () => { it("should list active chat rooms", async () => { const { stdout } = await runCommand(["rooms:list"], import.meta.url); @@ -160,6 +150,31 @@ describe("rooms:list command", () => { }); describe("error handling", () => { + it("should handle non-200 response with error", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ statusCode: 400, error: "Bad Request" }); + + const { error } = await runCommand(["rooms:list"], import.meta.url); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Failed to list rooms"); + }); + + it("should surface errorCode and errorMessage from HTTP response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + statusCode: 403, + errorCode: 40160, + errorMessage: "Action not permitted", + }); + + const { error } = await runCommand(["rooms:list"], import.meta.url); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Action not permitted"); + expect(error?.message).toContain("40160"); + }); + it("should handle API request failure gracefully", async () => { const mock = getMockAblyRest(); mock.request.mockRejectedValue(new Error("Network error")); diff --git a/test/unit/commands/rooms/typing/keystroke.test.ts b/test/unit/commands/rooms/typing/keystroke.test.ts index c251ce53..4b56804b 100644 --- a/test/unit/commands/rooms/typing/keystroke.test.ts +++ b/test/unit/commands/rooms/typing/keystroke.test.ts @@ -117,5 +117,45 @@ describe("rooms:typing:keystroke command", () => { expect(error).toBeDefined(); expect(error?.message).toContain("Room unavailable"); }); + + it("should handle keystroke failure gracefully without a stack trace", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + room.typing.keystroke.mockRejectedValue( + new Error("Channel denied access based on given capability"), + ); + + const { error, stderr } = await runCommand( + ["rooms:typing:keystroke", "test-room"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Channel denied access"); + // Must not dump a raw stack trace — the error should be caught cleanly + expect(stderr).not.toContain("at "); + expect(stderr).not.toContain("CLIError"); + }); + + it("should output JSON error on keystroke failure", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + room.typing.keystroke.mockRejectedValue( + new Error("Channel denied access based on given capability"), + ); + + const { stdout, stderr } = await runCommand( + ["rooms:typing:keystroke", "test-room", "--json"], + import.meta.url, + ); + + const result = JSON.parse(stdout); + expect(result).toHaveProperty("success", false); + expect(result).toHaveProperty("error"); + expect(result.error.message).toContain("Channel denied access"); + // Must not dump a raw stack trace — the error should be caught cleanly + expect(stderr).not.toContain("at "); + expect(stderr).not.toContain("CLIError"); + }); }); }); diff --git a/test/unit/commands/spaces/get.test.ts b/test/unit/commands/spaces/get.test.ts index f07b293f..53d1589b 100644 --- a/test/unit/commands/spaces/get.test.ts +++ b/test/unit/commands/spaces/get.test.ts @@ -178,5 +178,24 @@ describe("spaces:get command", () => { expect(error).toBeDefined(); expect(error?.message).toContain("500"); }); + + it("should surface errorCode and errorMessage from HTTP response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + items: [], + statusCode: 404, + errorCode: 40400, + errorMessage: "No application found with id test", + }); + + const { error } = await runCommand( + ["spaces:get", "test-space"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("No application found with id test"); + expect(error?.message).toContain("40400"); + }); }); }); diff --git a/test/unit/commands/spaces/list.test.ts b/test/unit/commands/spaces/list.test.ts index 72345867..e94fcf7d 100644 --- a/test/unit/commands/spaces/list.test.ts +++ b/test/unit/commands/spaces/list.test.ts @@ -192,5 +192,20 @@ describe("spaces:list command", () => { const { error } = await runCommand(["spaces:list"], import.meta.url); expect(error).toBeDefined(); }); + + it("should surface errorCode and errorMessage from HTTP response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + statusCode: 401, + errorCode: 40101, + errorMessage: "Invalid credentials", + }); + + const { error } = await runCommand(["spaces:list"], import.meta.url); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid credentials"); + expect(error?.message).toContain("40101"); + }); }); }); diff --git a/test/unit/commands/spaces/occupancy/get.test.ts b/test/unit/commands/spaces/occupancy/get.test.ts index 376c4c36..3cd560eb 100644 --- a/test/unit/commands/spaces/occupancy/get.test.ts +++ b/test/unit/commands/spaces/occupancy/get.test.ts @@ -12,6 +12,7 @@ describe("spaces:occupancy:get command", () => { beforeEach(() => { const mock = getMockAblyRest(); mock.request.mockResolvedValue({ + statusCode: 200, items: [ { status: { @@ -95,6 +96,7 @@ describe("spaces:occupancy:get command", () => { it("should handle empty occupancy metrics", async () => { const mock = getMockAblyRest(); mock.request.mockResolvedValue({ + statusCode: 200, items: [{}], }); @@ -124,5 +126,42 @@ describe("spaces:occupancy:get command", () => { expect(error).toBeDefined(); }); + + it("should handle HTTP error responses from API", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + success: false, + statusCode: 401, + errorCode: 40101, + errorMessage: "Invalid credentials", + }); + + const { error } = await runCommand( + ["spaces:occupancy:get", "test-space"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid credentials"); + }); + + it("should surface errorCode and errorMessage from HTTP response", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ + success: false, + statusCode: 404, + errorCode: 40400, + errorMessage: "Not found", + }); + + const { error } = await runCommand( + ["spaces:occupancy:get", "test-space"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Not found"); + expect(error?.message).toContain("40400"); + }); }); }); diff --git a/test/unit/errors/command-error.test.ts b/test/unit/errors/command-error.test.ts index 9eea9ec6..f7dd7bef 100644 --- a/test/unit/errors/command-error.test.ts +++ b/test/unit/errors/command-error.test.ts @@ -107,6 +107,40 @@ describe("CommandError", () => { expect(result.cause).toBeUndefined(); }); + it("should extract message and code from plain error object", () => { + const plainError = { message: "Invalid message format", code: 40000 }; + const result = CommandError.from(plainError); + expect(result.message).toBe("Invalid message format"); + expect(result.code).toBe(40000); + expect(result.statusCode).toBeUndefined(); + }); + + it("should extract message, code, and statusCode from plain error object", () => { + const plainError = { + message: "Unauthorized", + code: 40101, + statusCode: 401, + }; + const result = CommandError.from(plainError); + expect(result.message).toBe("Unauthorized"); + expect(result.code).toBe(40101); + expect(result.statusCode).toBe(401); + }); + + it("should handle plain object with message only", () => { + const plainError = { message: "Something went wrong" }; + const result = CommandError.from(plainError); + expect(result.message).toBe("Something went wrong"); + expect(result.code).toBeUndefined(); + expect(result.statusCode).toBeUndefined(); + }); + + it("should not treat objects without message as plain error objects", () => { + const notAnError = { code: 123, data: "test" }; + const result = CommandError.from(notAnError); + expect(result.message).toBe("[object Object]"); + }); + it("should wrap unknown values via String()", () => { const result = CommandError.from(42); expect(result.message).toBe("42"); @@ -185,6 +219,67 @@ describe("CommandError", () => { }); }); + describe("fromHttpResponse()", () => { + it("should extract errorCode, errorMessage, and statusCode", () => { + const response = { + statusCode: 401, + errorCode: 40101, + errorMessage: "Invalid credentials", + }; + const result = CommandError.fromHttpResponse( + response, + "Failed to list channels", + ); + expect(result.message).toBe("Invalid credentials"); + expect(result.code).toBe(40101); + expect(result.statusCode).toBe(401); + }); + + it("should fall back to action message when errorMessage is empty", () => { + const response = { + statusCode: 500, + errorCode: 0, + errorMessage: "", + }; + const result = CommandError.fromHttpResponse( + response, + "Failed to list spaces", + ); + expect(result.message).toBe("Failed to list spaces (status 500)"); + expect(result.code).toBeUndefined(); + expect(result.statusCode).toBe(500); + }); + + it("should set code to undefined when errorCode is 0", () => { + const response = { + statusCode: 500, + errorCode: 0, + errorMessage: "Internal server error", + }; + const result = CommandError.fromHttpResponse( + response, + "Failed to list rooms", + ); + expect(result.message).toBe("Internal server error"); + expect(result.code).toBeUndefined(); + expect(result.statusCode).toBe(500); + }); + + it("should preserve non-zero errorCode", () => { + const response = { + statusCode: 403, + errorCode: 40160, + errorMessage: "Action not permitted", + }; + const result = CommandError.fromHttpResponse( + response, + "Failed to fetch space", + ); + expect(result.code).toBe(40160); + expect(result.statusCode).toBe(403); + }); + }); + describe("toJsonData()", () => { it("should nest error data under error key with message field", () => { const err = new CommandError("test error");