From cd5760a50d34296317a5f3c840f6d5a91994328a Mon Sep 17 00:00:00 2001 From: umair Date: Fri, 13 Mar 2026 02:43:08 +0000 Subject: [PATCH] Improve error code formatting --- src/base-command.ts | 18 +++++- src/commands/channels/occupancy/subscribe.ts | 2 +- src/commands/channels/presence/enter.ts | 2 +- src/commands/channels/presence/subscribe.ts | 60 ++++++++++--------- src/commands/channels/subscribe.ts | 22 ++----- .../logs/channel-lifecycle/subscribe.ts | 2 +- .../logs/connection-lifecycle/subscribe.ts | 2 +- src/commands/logs/push/subscribe.ts | 2 +- src/commands/logs/subscribe.ts | 59 +++++++++--------- src/utils/errors.ts | 23 +++++++ .../channels/occupancy/subscribe.test.ts | 26 ++++++++ .../channels/presence/subscribe.test.ts | 26 ++++++++ test/unit/commands/channels/subscribe.test.ts | 26 ++++++++ test/unit/utils/errors.test.ts | 59 ++++++++++++++++++ 14 files changed, 249 insertions(+), 80 deletions(-) create mode 100644 test/unit/utils/errors.test.ts diff --git a/src/base-command.ts b/src/base-command.ts index 9d96ba79..eed98004 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -10,6 +10,7 @@ import { } from "./services/config-manager.js"; import { ControlApi } from "./services/control-api.js"; import { CommandError } from "./errors/command-error.js"; +import { getFriendlyAblyErrorHint } from "./utils/errors.js"; import { coreGlobalFlags } from "./flags.js"; import { InteractiveHelper } from "./services/interactive-helper.js"; import { BaseFlags, CommandConfig } from "./types/cli.js"; @@ -936,9 +937,10 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { logData, ); } else if (level <= 1) { - // Standard Non-JSON: Log only SDK ERRORS (level <= 1) clearly - // Use a format similar to logCliEvent's non-JSON output - this.log(`${chalk.red.bold(`[AblySDK Error]`)} ${message}`); + // 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}`); } // If not verbose non-JSON and level > 1, suppress non-error SDK logs } @@ -1503,6 +1505,16 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { } let humanMessage = cmdError.message; + const friendlyHint = getFriendlyAblyErrorHint( + cmdError.code ?? + (typeof cmdError.context.errorCode === "number" + ? cmdError.context.errorCode + : undefined), + ); + if (friendlyHint) { + humanMessage += `\n${friendlyHint}`; + } + const code = cmdError.code ?? cmdError.context.errorCode; if (code !== undefined) { const helpUrl = cmdError.context.helpUrl; diff --git a/src/commands/channels/occupancy/subscribe.ts b/src/commands/channels/occupancy/subscribe.ts index 67f56f0e..7e393e16 100644 --- a/src/commands/channels/occupancy/subscribe.ts +++ b/src/commands/channels/occupancy/subscribe.ts @@ -86,7 +86,7 @@ export default class ChannelsOccupancySubscribe extends AblyBaseCommand { ); } - channel.subscribe(occupancyEventName, (message: Ably.Message) => { + await channel.subscribe(occupancyEventName, (message: Ably.Message) => { const timestamp = formatMessageTimestamp(message.timestamp); const event = { channel: channelName, diff --git a/src/commands/channels/presence/enter.ts b/src/commands/channels/presence/enter.ts index 7635040f..b5288646 100644 --- a/src/commands/channels/presence/enter.ts +++ b/src/commands/channels/presence/enter.ts @@ -104,7 +104,7 @@ export default class ChannelsPresenceEnter extends AblyBaseCommand { // Subscribe to presence events before entering (if show-others is enabled) if (flags["show-others"]) { - this.channel.presence.subscribe((presenceMessage) => { + await this.channel.presence.subscribe((presenceMessage) => { // Filter out own presence events if (presenceMessage.clientId === client.auth.clientId) { return; diff --git a/src/commands/channels/presence/subscribe.ts b/src/commands/channels/presence/subscribe.ts index dfb23259..91c408dd 100644 --- a/src/commands/channels/presence/subscribe.ts +++ b/src/commands/channels/presence/subscribe.ts @@ -79,41 +79,43 @@ export default class ChannelsPresenceSubscribe extends AblyBaseCommand { ); } - channel.presence.subscribe((presenceMessage: Ably.PresenceMessage) => { - const timestamp = formatMessageTimestamp(presenceMessage.timestamp); - const presenceData = { - id: presenceMessage.id, - timestamp, - action: presenceMessage.action, - channel: channelName, - clientId: presenceMessage.clientId, - connectionId: presenceMessage.connectionId, - data: presenceMessage.data, - }; - this.logCliEvent( - flags, - "presence", - presenceMessage.action!, - `Presence event: ${presenceMessage.action} by ${presenceMessage.clientId}`, - presenceData, - ); - - if (this.shouldOutputJson(flags)) { - this.logJsonEvent({ presenceMessage: presenceData }, flags); - } else { - const displayFields: PresenceDisplayFields = { + await channel.presence.subscribe( + (presenceMessage: Ably.PresenceMessage) => { + const timestamp = formatMessageTimestamp(presenceMessage.timestamp); + const presenceData = { id: presenceMessage.id, - timestamp: presenceMessage.timestamp ?? Date.now(), - action: presenceMessage.action || "unknown", + timestamp, + action: presenceMessage.action, channel: channelName, clientId: presenceMessage.clientId, connectionId: presenceMessage.connectionId, data: presenceMessage.data, }; - this.log(formatPresenceOutput([displayFields])); - this.log(""); // Empty line for better readability - } - }); + this.logCliEvent( + flags, + "presence", + presenceMessage.action!, + `Presence event: ${presenceMessage.action} by ${presenceMessage.clientId}`, + presenceData, + ); + + if (this.shouldOutputJson(flags)) { + this.logJsonEvent({ presenceMessage: presenceData }, flags); + } else { + const displayFields: PresenceDisplayFields = { + id: presenceMessage.id, + timestamp: presenceMessage.timestamp ?? Date.now(), + action: presenceMessage.action || "unknown", + channel: channelName, + clientId: presenceMessage.clientId, + connectionId: presenceMessage.connectionId, + data: presenceMessage.data, + }; + this.log(formatPresenceOutput([displayFields])); + this.log(""); // Empty line for better readability + } + }, + ); if (!this.shouldOutputJson(flags)) { this.log( diff --git a/src/commands/channels/subscribe.ts b/src/commands/channels/subscribe.ts index 9c899a18..7990b87a 100644 --- a/src/commands/channels/subscribe.ts +++ b/src/commands/channels/subscribe.ts @@ -154,7 +154,7 @@ export default class ChannelsSubscribe extends AblyBaseCommand { }); // Subscribe to messages on all channels - const attachPromises: Promise[] = []; + const subscribePromises: Promise[] = []; for (const channel of channels) { this.logCliEvent( @@ -177,19 +177,8 @@ export default class ChannelsSubscribe extends AblyBaseCommand { includeUserFriendlyMessages: true, }); - // Track attachment promise - const attachPromise = new Promise((resolve) => { - const checkAttached = () => { - if (channel.state === "attached") { - resolve(); - } - }; - channel.once("attached", checkAttached); - checkAttached(); // Check if already attached - }); - attachPromises.push(attachPromise); - - channel.subscribe((message: Ably.Message) => { + // Subscribe and collect promise (rejects on capability/auth errors) + const subscribePromise = channel.subscribe((message: Ably.Message) => { this.sequenceCounter++; const timestamp = formatMessageTimestamp(message.timestamp); const messageData = { @@ -251,10 +240,11 @@ export default class ChannelsSubscribe extends AblyBaseCommand { this.log(""); // Empty line for readability between messages } }); + subscribePromises.push(subscribePromise); } - // Wait for all channels to attach - await Promise.all(attachPromises); + // Wait for all channels to attach via subscribe + await Promise.all(subscribePromises); // Log the ready signal for E2E tests if (channelNames.length === 1 && !this.shouldOutputJson(flags)) { diff --git a/src/commands/logs/channel-lifecycle/subscribe.ts b/src/commands/logs/channel-lifecycle/subscribe.ts index 1731068b..b3317ee8 100644 --- a/src/commands/logs/channel-lifecycle/subscribe.ts +++ b/src/commands/logs/channel-lifecycle/subscribe.ts @@ -88,7 +88,7 @@ export default class LogsChannelLifecycleSubscribe extends AblyBaseCommand { } // Subscribe to the channel - channel.subscribe((message) => { + await channel.subscribe((message) => { const timestamp = formatMessageTimestamp(message.timestamp); const event = message.name || "unknown"; const logEvent = { diff --git a/src/commands/logs/connection-lifecycle/subscribe.ts b/src/commands/logs/connection-lifecycle/subscribe.ts index c77d4d3e..62668f59 100644 --- a/src/commands/logs/connection-lifecycle/subscribe.ts +++ b/src/commands/logs/connection-lifecycle/subscribe.ts @@ -85,7 +85,7 @@ export default class LogsConnectionLifecycleSubscribe extends AblyBaseCommand { } // Subscribe to connection lifecycle logs - channel.subscribe((message: Ably.Message) => { + await channel.subscribe((message: Ably.Message) => { const timestamp = formatMessageTimestamp(message.timestamp); const event = { timestamp, diff --git a/src/commands/logs/push/subscribe.ts b/src/commands/logs/push/subscribe.ts index 2e63b50a..b7eb86ce 100644 --- a/src/commands/logs/push/subscribe.ts +++ b/src/commands/logs/push/subscribe.ts @@ -79,7 +79,7 @@ export default class LogsPushSubscribe extends AblyBaseCommand { ); // Subscribe to the channel - channel.subscribe((message) => { + await channel.subscribe((message) => { const timestamp = formatMessageTimestamp(message.timestamp); const event = message.name || "unknown"; const logEvent = { diff --git a/src/commands/logs/subscribe.ts b/src/commands/logs/subscribe.ts index 7b2ba873..5f2b3d61 100644 --- a/src/commands/logs/subscribe.ts +++ b/src/commands/logs/subscribe.ts @@ -120,41 +120,46 @@ export default class LogsSubscribe extends AblyBaseCommand { } // Subscribe to specified log types + const subscribePromises: Promise[] = []; for (const logType of logTypes) { - channel.subscribe(logType, (message: Ably.Message) => { - const timestamp = formatMessageTimestamp(message.timestamp); - const event = { - logType, - timestamp, - data: message.data, - id: message.id, - }; - this.logCliEvent( - flags, - "logs", - "logReceived", - `Log received: ${logType}`, - event, - ); - - if (this.shouldOutputJson(flags)) { - this.logJsonEvent(event, flags); - } else { - this.log( - `${formatTimestamp(timestamp)} Type: ${formatEventType(logType)}`, + subscribePromises.push( + channel.subscribe(logType, (message: Ably.Message) => { + const timestamp = formatMessageTimestamp(message.timestamp); + const event = { + logType, + timestamp, + data: message.data, + id: message.id, + }; + this.logCliEvent( + flags, + "logs", + "logReceived", + `Log received: ${logType}`, + event, ); - if (message.data !== null && message.data !== undefined) { + if (this.shouldOutputJson(flags)) { + this.logJsonEvent(event, flags); + } else { this.log( - `${formatLabel("Data")} ${JSON.stringify(message.data, null, 2)}`, + `${formatTimestamp(timestamp)} Type: ${formatEventType(logType)}`, ); - } - this.log(""); // Empty line for better readability - } - }); + if (message.data !== null && message.data !== undefined) { + this.log( + `${formatLabel("Data")} ${JSON.stringify(message.data, null, 2)}`, + ); + } + + this.log(""); // Empty line for better readability + } + }), + ); } + await Promise.all(subscribePromises); + this.logCliEvent( flags, "logs", diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 2b963077..eb08dc14 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -4,3 +4,26 @@ export function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } + +/** + * Return a friendly, actionable hint for known Ably error codes. + * Returns undefined for unknown codes. + */ +const hints: Record = { + 40101: + 'The credentials provided are not valid. Check your API key or token, or re-authenticate with "ably login".', + 40103: 'The token has expired. Please re-authenticate with "ably login".', + 40110: + 'Unable to authorize. Check your authentication configuration or re-authenticate with "ably login".', + 40160: + "The current credentials do not have the capability to perform this operation on the requested resource. Check the token or key capability configuration in the Ably dashboard.", + 40161: + "The current credentials do not have publish capability on this resource. Check the token or key capability configuration in the Ably dashboard.", + 40171: + "The requested operation is not permitted by the current credentials' capabilities. Check the token or key capability configuration in the Ably dashboard.", +}; + +export function getFriendlyAblyErrorHint(code?: number): string | undefined { + if (code === undefined) return undefined; + return hints[code]; +} diff --git a/test/unit/commands/channels/occupancy/subscribe.test.ts b/test/unit/commands/channels/occupancy/subscribe.test.ts index 032a210d..0351567d 100644 --- a/test/unit/commands/channels/occupancy/subscribe.test.ts +++ b/test/unit/commands/channels/occupancy/subscribe.test.ts @@ -118,6 +118,32 @@ describe("channels:occupancy:subscribe command", () => { expect(error).toBeDefined(); expect(error?.message).toMatch(/No mock|client/i); }); + + it("should handle capability error gracefully", async () => { + const mock = getMockAblyRealtime(); + const channel = mock.channels._getChannel("test-channel"); + + channel.subscribe.mockRejectedValue( + Object.assign( + new Error("Channel denied access based on given capability"), + { + code: 40160, + statusCode: 401, + href: "https://help.ably.io/error/40160", + }, + ), + ); + + const { error } = await runCommand( + ["channels:occupancy:subscribe", "test-channel"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Channel denied access"); + expect(error?.message).toContain("capability"); + expect(error?.message).toContain("Ably dashboard"); + }); }); describe("output formats", () => { diff --git a/test/unit/commands/channels/presence/subscribe.test.ts b/test/unit/commands/channels/presence/subscribe.test.ts index a1e240f5..0d20210f 100644 --- a/test/unit/commands/channels/presence/subscribe.test.ts +++ b/test/unit/commands/channels/presence/subscribe.test.ts @@ -183,5 +183,31 @@ describe("channels:presence:subscribe command", () => { expect(error).toBeDefined(); expect(error?.message).toMatch(/No mock|client/i); }); + + it("should handle capability error gracefully", async () => { + const mock = getMockAblyRealtime(); + const channel = mock.channels._getChannel("test-channel"); + + channel.presence.subscribe.mockRejectedValue( + Object.assign( + new Error("Channel denied access based on given capability"), + { + code: 40160, + statusCode: 401, + href: "https://help.ably.io/error/40160", + }, + ), + ); + + const { error } = await runCommand( + ["channels:presence:subscribe", "test-channel"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Channel denied access"); + expect(error?.message).toContain("capability"); + expect(error?.message).toContain("Ably dashboard"); + }); }); }); diff --git a/test/unit/commands/channels/subscribe.test.ts b/test/unit/commands/channels/subscribe.test.ts index a3fa5b20..f0bfa76b 100644 --- a/test/unit/commands/channels/subscribe.test.ts +++ b/test/unit/commands/channels/subscribe.test.ts @@ -239,5 +239,31 @@ describe("channels:subscribe command", () => { expect(error).toBeDefined(); expect(error?.message).toMatch(/No mock|client/i); }); + + it("should handle capability error gracefully", async () => { + const mock = getMockAblyRealtime(); + const channel = mock.channels._getChannel("test-channel"); + + channel.subscribe.mockRejectedValue( + Object.assign( + new Error("Channel denied access based on given capability"), + { + code: 40160, + statusCode: 401, + href: "https://help.ably.io/error/40160", + }, + ), + ); + + const { error } = await runCommand( + ["channels:subscribe", "test-channel"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Channel denied access"); + expect(error?.message).toContain("capability"); + expect(error?.message).toContain("Ably dashboard"); + }); }); }); diff --git a/test/unit/utils/errors.test.ts b/test/unit/utils/errors.test.ts new file mode 100644 index 00000000..7ce67e70 --- /dev/null +++ b/test/unit/utils/errors.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from "vitest"; +import { + errorMessage, + getFriendlyAblyErrorHint, +} from "../../../src/utils/errors.js"; + +describe("errorMessage", () => { + it("should extract message from Error instances", () => { + expect(errorMessage(new Error("test error"))).toBe("test error"); + }); + + it("should stringify non-Error values", () => { + expect(errorMessage("string error")).toBe("string error"); + expect(errorMessage(42)).toBe("42"); + }); +}); + +describe("getFriendlyAblyErrorHint", () => { + it("should return capability hint for code 40160", () => { + const hint = getFriendlyAblyErrorHint(40160); + expect(hint).toContain("capability"); + expect(hint).toContain("Ably dashboard"); + }); + + it("should return publish capability hint for code 40161", () => { + const hint = getFriendlyAblyErrorHint(40161); + expect(hint).toContain("publish capability"); + }); + + it("should return operation not permitted hint for code 40171", () => { + const hint = getFriendlyAblyErrorHint(40171); + expect(hint).toContain("not permitted"); + }); + + it("should return invalid credentials hint for code 40101", () => { + const hint = getFriendlyAblyErrorHint(40101); + expect(hint).toContain("not valid"); + expect(hint).toContain("ably login"); + }); + + it("should return token expired hint for code 40103", () => { + const hint = getFriendlyAblyErrorHint(40103); + expect(hint).toContain("expired"); + expect(hint).toContain("ably login"); + }); + + it("should return unable to authorize hint for code 40110", () => { + const hint = getFriendlyAblyErrorHint(40110); + expect(hint).toContain("Unable to authorize"); + }); + + it("should return undefined for unknown error codes", () => { + expect(getFriendlyAblyErrorHint(99999)).toBeUndefined(); + }); + + it("should return undefined when code is not provided", () => { + expect(getFriendlyAblyErrorHint()).toBeUndefined(); + }); +});