Skip to content

Commit c646bbd

Browse files
mattheworiordanmattheworiordan
committed
ci: Fix test failures
Includes a workaround for a bug in Spaces, see ably/spaces#339
1 parent a34d987 commit c646bbd

File tree

15 files changed

+873
-529
lines changed

15 files changed

+873
-529
lines changed

‎README.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4262,12 +4262,14 @@ Set a cursor with position data in a space
42624262
```
42634263
USAGE
42644264
$ ably spaces cursors set SPACEID --data <value> [--access-token <value>] [--api-key <value>] [--client-id <value>]
4265-
[--env <value>] [--host <value>] [--json | --pretty-json] [--token <value>] [-v]
4265+
[--env <value>] [--host <value>] [--json | --pretty-json] [--token <value>] [-v] [-D <value>]
42664266
42674267
ARGUMENTS
42684268
SPACEID The space ID to set cursor in
42694269
42704270
FLAGS
4271+
-D, --duration=<value> Automatically exit after the given number of seconds (0 = exit immediately after setting
4272+
the cursor)
42714273
-v, --verbose Output verbose logs
42724274
--access-token=<value> Overrides any configured access token used for the Control API
42734275
--api-key=<value> Overrides any configured API key used for the product APIs
@@ -4450,12 +4452,14 @@ Set your location in a space
44504452
```
44514453
USAGE
44524454
$ ably spaces locations set SPACEID --location <value> [--access-token <value>] [--api-key <value>] [--client-id
4453-
<value>] [--env <value>] [--host <value>] [--json | --pretty-json] [--token <value>] [-v]
4455+
<value>] [--env <value>] [--host <value>] [--json | --pretty-json] [--token <value>] [-v] [-D <value>]
44544456
44554457
ARGUMENTS
44564458
SPACEID Space ID to set location in
44574459
44584460
FLAGS
4461+
-D, --duration=<value> Automatically exit after the given number of seconds (0 = exit immediately after setting
4462+
location)
44594463
-v, --verbose Output verbose logs
44604464
--access-token=<value> Overrides any configured access token used for the Control API
44614465
--api-key=<value> Overrides any configured API key used for the product APIs

‎src/commands/channels/presence/enter.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,13 @@ export default class ChannelsPresenceEnter extends AblyBaseCommand {
8484
let profileData: any = undefined;
8585
if (flags["profile-data"]) {
8686
try {
87-
profileData = JSON.parse(flags["profile-data"]);
87+
let trimmed = (flags["profile-data"] as string).trim();
88+
if ((trimmed.startsWith("'") && trimmed.endsWith("'")) || (trimmed.startsWith('"') && trimmed.endsWith('"'))) {
89+
trimmed = trimmed.slice(1, -1);
90+
}
91+
profileData = JSON.parse(trimmed);
8892
} catch (error) {
89-
const errorMsg = `Invalid JSON in profile-data: ${error instanceof Error ? error.message : String(error)}`;
93+
const errorMsg = `Invalid profile-data or data JSON: ${error instanceof Error ? error.message : String(error)}`;
9094
this.logCliEvent(
9195
flags,
9296
"presence",

‎src/commands/rooms/messages/send.ts‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -489,23 +489,7 @@ export default class MessagesSend extends ChatBaseCommand {
489489
this.error(`Failed to send message: ${errorMsg}`);
490490
}
491491
} finally {
492-
// Proper cleanup sequence
493-
try {
494-
// Release room if we haven't already
495-
if (this.chatClient && this.roomId) {
496-
await this.chatClient.rooms.release(this.roomId);
497-
}
498-
} catch {
499-
// Ignore release errors in cleanup
500-
}
501-
502-
// Clear any remaining intervals
503-
if (this.progressIntervalId) {
504-
clearInterval(this.progressIntervalId);
505-
this.progressIntervalId = null;
506-
}
507-
508-
// Close Ably client properly - this is now handled in finally() override
492+
// Cleanup is handled in the finally() override method to avoid duplication
509493
}
510494
}
511495

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

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -140,28 +140,40 @@ export default class MessagesSubscribe extends ChatBaseCommand {
140140
// Close Ably client properly with timeout
141141
await this.properlyCloseAblyClient();
142142

143-
return super.finally(err);
143+
// Ensure the process does not linger due to any stray async handles
144+
await super.finally(err);
145+
146+
// Force a graceful exit shortly after cleanup to avoid hanging (skip in tests)
147+
if (process.env.NODE_ENV !== 'test') {
148+
setTimeout(() => {
149+
process.exit(0);
150+
}, 100);
151+
}
144152
}
145153

146154
async run(): Promise<void> {
147155
const { args, flags } = await this.parse(MessagesSubscribe);
148156
this.roomId = args.roomId; // Store for cleanup
157+
this.logCliEvent(flags, "subscribe.run", "start", `Starting rooms messages subscribe for room: ${this.roomId}`);
149158

150159
try {
151160
// Always show the readiness signal first, before attempting auth
152161
if (!this.shouldOutputJson(flags)) {
153162
this.log(`${chalk.dim("Listening for messages. Press Ctrl+C to exit.")}`);
154163
}
164+
this.logCliEvent(flags, "subscribe.run", "initialSignalLogged", "Initial readiness signal logged.");
155165

156166
// Try to create clients, but don't fail if auth fails
157167
try {
168+
this.logCliEvent(flags, "subscribe.auth", "attemptingClientCreation", "Attempting to create Chat and Ably clients.");
158169
// Create Chat client
159170
this.chatClient = await this.createChatClient(flags) as ChatClientType;
160171
// Also get the underlying Ably client for cleanup and state listeners
161172
this.ablyClient = await this.createAblyClient(flags);
173+
this.logCliEvent(flags, "subscribe.auth", "clientCreationSuccess", "Chat and Ably clients created.");
162174
} catch (authError) {
163-
// Auth failed, but we still want to show the signal and wait
164-
this.logCliEvent(flags, "initialization", "authFailed", `Authentication failed: ${authError instanceof Error ? authError.message : String(authError)}`);
175+
const errorMsg = authError instanceof Error ? authError.message : String(authError);
176+
this.logCliEvent(flags, "initialization", "authFailed", `Authentication failed: ${errorMsg}`, { error: errorMsg });
165177
if (!this.shouldOutputJson(flags)) {
166178
this.log(`Connected to room: ${this.roomId || args.roomId} (mock)`);
167179
this.log("Listening for messages");
@@ -242,19 +254,9 @@ export default class MessagesSubscribe extends ChatBaseCommand {
242254
);
243255

244256
// Get the room
245-
this.logCliEvent(
246-
flags,
247-
"room",
248-
"gettingRoom",
249-
`Getting room handle for ${this.roomId}`,
250-
);
257+
this.logCliEvent(flags, "room", "gettingRoom", `Getting room handle for ${this.roomId}`);
251258
const room = await this.chatClient.rooms.get(this.roomId, {});
252-
this.logCliEvent(
253-
flags,
254-
"room",
255-
"gotRoom",
256-
`Got room handle for ${this.roomId}`,
257-
);
259+
this.logCliEvent(flags, "room", "gotRoom", `Got room handle for ${this.roomId}`);
258260

259261
// Setup message handler
260262
this.logCliEvent(
@@ -317,24 +319,13 @@ export default class MessagesSubscribe extends ChatBaseCommand {
317319
);
318320

319321
// Subscribe to room status changes
320-
this.logCliEvent(
321-
flags,
322-
"room",
323-
"subscribingToStatus",
324-
`Subscribing to status changes for room ${this.roomId}`,
325-
);
322+
this.logCliEvent(flags, "room", "subscribingToStatus", `Subscribing to status changes for room ${this.roomId}`);
326323
this.unsubscribeStatusFn = room.onStatusChange(
327324
(statusChange: unknown) => {
328-
// Type assertion after we receive it
329325
const change = statusChange as StatusChange;
330-
this.logCliEvent(
331-
flags,
332-
"room",
333-
`status-${change.current}`,
334-
`Room status changed to ${change.current}`,
335-
{ reason: change.reason, roomId: this.roomId },
336-
);
326+
this.logCliEvent(flags, "room", `status-${change.current}`, `Room status changed to ${change.current}`, { reason: change.reason, roomId: this.roomId });
337327
if (change.current === "attached") {
328+
this.logCliEvent(flags, "room", "statusAttached", "Room status is ATTACHED.");
338329
if (!this.shouldSuppressOutput(flags)) {
339330
if (this.shouldOutputJson(flags)) {
340331
// Already logged via logCliEvent
@@ -344,6 +335,7 @@ export default class MessagesSubscribe extends ChatBaseCommand {
344335
);
345336
// Output the exact signal that E2E tests expect (without ANSI codes)
346337
this.log("Listening for messages");
338+
this.logCliEvent(flags, "room", "readySignalLogged", "Final readiness signal 'Listening for messages' logged.");
347339
this.log(
348340
`${chalk.dim("Press Ctrl+C to exit.")}`,
349341
);
@@ -368,13 +360,9 @@ export default class MessagesSubscribe extends ChatBaseCommand {
368360
);
369361

370362
// Attach to the room
371-
this.logCliEvent(
372-
flags,
373-
"room",
374-
"attaching",
375-
`Attaching to room ${this.roomId}`,
376-
);
363+
this.logCliEvent(flags, "room", "attaching", `Attaching to room ${this.roomId}`);
377364
await room.attach();
365+
this.logCliEvent(flags, "room", "attachCallComplete", `room.attach() call complete for ${this.roomId}. Waiting for status change to 'attached'.`);
378366
// Note: successful attach logged by onStatusChange handler
379367

380368
this.logCliEvent(

‎src/commands/rooms/occupancy/get.ts‎

Lines changed: 78 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -26,30 +26,65 @@ export default class RoomsOccupancyGet extends ChatBaseCommand {
2626

2727
private ablyClient: Ably.Realtime | null = null;
2828
private chatClient: ChatClient | null = null;
29+
private room: any = null;
2930

30-
private async properlyCloseAblyClient(): Promise<void> {
31-
if (!this.ablyClient || this.ablyClient.connection.state === 'closed') {
32-
return;
31+
private async forceCloseConnections(): Promise<void> {
32+
try {
33+
// First try to release the room
34+
if (this.room) {
35+
await Promise.race([
36+
this.room.detach(),
37+
new Promise(resolve => setTimeout(resolve, 1000)) // 1s timeout
38+
]);
39+
}
40+
} catch {
41+
// Ignore detach errors
3342
}
3443

35-
return new Promise<void>((resolve) => {
36-
const timeout = setTimeout(() => {
37-
console.warn('Ably client cleanup timed out after 3 seconds');
38-
resolve();
39-
}, 3000);
40-
41-
const onClosed = () => {
42-
clearTimeout(timeout);
43-
resolve();
44-
};
44+
try {
45+
// Release room from chat client
46+
if (this.chatClient && this.room) {
47+
await Promise.race([
48+
this.chatClient.rooms.release(this.room.roomId),
49+
new Promise(resolve => setTimeout(resolve, 1000)) // 1s timeout
50+
]);
51+
}
52+
} catch {
53+
// Ignore release errors
54+
}
4555

46-
// Listen for both closed and failed states
47-
this.ablyClient!.connection.once('closed', onClosed);
48-
this.ablyClient!.connection.once('failed', onClosed);
49-
50-
// Close the client
51-
this.ablyClient!.close();
52-
});
56+
try {
57+
// Force close the Ably client
58+
if (this.ablyClient) {
59+
await Promise.race([
60+
new Promise<void>((resolve) => {
61+
if (this.ablyClient!.connection.state === 'closed') {
62+
resolve();
63+
return;
64+
}
65+
66+
const onClosed = () => {
67+
resolve();
68+
};
69+
70+
// Listen for closed and failed states
71+
this.ablyClient!.connection.once('closed', onClosed);
72+
this.ablyClient!.connection.once('failed', onClosed);
73+
this.ablyClient!.close();
74+
75+
// Cleanup listeners after 2 seconds
76+
setTimeout(() => {
77+
this.ablyClient!.connection.off('closed', onClosed);
78+
this.ablyClient!.connection.off('failed', onClosed);
79+
resolve();
80+
}, 2000);
81+
}),
82+
new Promise<void>(resolve => setTimeout(resolve, 2000)) // 2s timeout
83+
]);
84+
}
85+
} catch {
86+
// Ignore close errors
87+
}
5388
}
5489

5590
async run(): Promise<void> {
@@ -69,13 +104,23 @@ export default class RoomsOccupancyGet extends ChatBaseCommand {
69104
const { roomId } = args;
70105

71106
// Get the room with occupancy enabled
72-
const room = await this.chatClient.rooms.get(roomId, {});
107+
this.room = await this.chatClient.rooms.get(roomId, {});
73108

74-
// Attach to the room to access occupancy
75-
await room.attach();
109+
// Attach to the room to access occupancy with timeout
110+
await Promise.race([
111+
this.room.attach(),
112+
new Promise((_, reject) =>
113+
setTimeout(() => reject(new Error('Room attach timeout')), 10000)
114+
)
115+
]);
76116

77117
// Get occupancy metrics using the Chat SDK's occupancy API
78-
const occupancyMetrics = await room.occupancy.get();
118+
const occupancyMetrics = await Promise.race([
119+
this.room.occupancy.get(),
120+
new Promise((_, reject) =>
121+
setTimeout(() => reject(new Error('Occupancy get timeout')), 5000)
122+
)
123+
]);
79124

80125
// Output the occupancy metrics based on format
81126
if (this.shouldOutputJson(flags)) {
@@ -93,13 +138,8 @@ export default class RoomsOccupancyGet extends ChatBaseCommand {
93138
this.log(`Occupancy metrics for room '${roomId}':\n`);
94139
this.log(`Connections: ${occupancyMetrics.connections ?? 0}`);
95140

96-
if (occupancyMetrics.presenceMembers !== undefined) {
97-
this.log(`Presence Members: ${occupancyMetrics.presenceMembers}`);
98-
}
141+
this.log(`Presence Members: ${occupancyMetrics.presenceMembers ?? 0}`);
99142
}
100-
101-
// Release the room BEFORE closing the client
102-
await this.chatClient.rooms.release(roomId);
103143

104144
} catch (error) {
105145
if (this.shouldOutputJson(flags)) {
@@ -119,18 +159,15 @@ export default class RoomsOccupancyGet extends ChatBaseCommand {
119159
);
120160
}
121161
} finally {
122-
// Proper cleanup sequence
123-
try {
124-
// Release room if we haven't already
125-
if (this.chatClient && args?.roomId) {
126-
await this.chatClient.rooms.release(args.roomId);
162+
// Force cleanup with timeouts to ensure the command exits
163+
await this.forceCloseConnections();
164+
165+
// Force exit after cleanup
166+
setTimeout(() => {
167+
if (process.env.NODE_ENV !== 'test') {
168+
process.exit(0);
127169
}
128-
} catch {
129-
// Ignore release errors in cleanup
130-
}
131-
132-
// Close Ably client properly
133-
await this.properlyCloseAblyClient();
170+
}, 100);
134171
}
135172
}
136173
}

‎src/commands/rooms/presence/enter.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,18 @@ export default class RoomsPresenceEnter extends ChatBaseCommand {
8383
this.commandFlags = flags;
8484
this.roomId = args.roomId;
8585

86-
const profileDataString = flags["profile-data"] || flags.data;
87-
if (profileDataString && profileDataString !== "{}") {
86+
const rawProfileData = flags["profile-data"] || flags.data;
87+
if (rawProfileData && rawProfileData !== "{}") {
8888
try {
89-
this.profileData = JSON.parse(profileDataString);
89+
let trimmed = rawProfileData.trim();
90+
// If the string is wrapped in single or double quotes (common when passed through a shell), remove them first.
91+
if ((trimmed.startsWith("'") && trimmed.endsWith("'")) || (trimmed.startsWith('"') && trimmed.endsWith('"'))) {
92+
trimmed = trimmed.slice(1, -1);
93+
}
94+
this.profileData = JSON.parse(trimmed);
9095
} catch (error) {
9196
this.error(`Invalid profile-data or data JSON: ${error instanceof Error ? error.message : String(error)}`);
92-
return;
97+
return; // Exit early if JSON is invalid
9398
}
9499
}
95100

0 commit comments

Comments
 (0)