Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure QUIT messages are sent in pooling configuration #1752

Merged
merged 8 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1752.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure QUIT messages are always sent.
2 changes: 1 addition & 1 deletion spec/e2e/basic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('Basic bridge usage', () => {
let testEnv: IrcBridgeE2ETest;
beforeEach(async () => {
testEnv = await IrcBridgeE2ETest.createTestEnv({
matrixLocalparts: ['alice'],
matrixLocalparts: [TestIrcServer.generateUniqueNick("alice")],
ircNicks: ['bob'],
traceToFile: true,
});
Expand Down
2 changes: 1 addition & 1 deletion spec/e2e/pooling.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describeif('Connection pooling', () => {
beforeEach(async () => {
// Initial run of the bridge to setup a testing environment
testEnv = await IrcBridgeE2ETest.createTestEnv({
matrixLocalparts: ['alice'],
matrixLocalparts: [TestIrcServer.generateUniqueNick('alice')],
ircNicks: ['bob'],
config: {
connectionPool: {
Expand Down
43 changes: 43 additions & 0 deletions spec/e2e/quit.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { TestIrcServer } from "matrix-org-irc";
import { IrcBridgeE2ETest } from "../util/e2e-test";
import { describe, it } from "@jest/globals";


describe('Ensure quit messsage is sent', () => {
let testEnv: IrcBridgeE2ETest;
beforeEach(async () => {
testEnv = await IrcBridgeE2ETest.createTestEnv({
matrixLocalparts: [TestIrcServer.generateUniqueNick('alice')],
ircNicks: ['bob'],
traceToFile: true,
});
await testEnv.setUp();
});
afterEach(() => {
return testEnv?.tearDown();
});
it('should correctly send a quit message on !reconnect', async () => {
const channel = `#${TestIrcServer.generateUniqueNick("test")}`;
const { homeserver, opts } = testEnv;
const [alice] = homeserver.users.map(c => c.client);
const { bob } = testEnv.ircTest.clients;

// Create the channel
await bob.join(channel);
const adminRoom = await testEnv.createAdminRoomHelper(alice)
const cRoomId = await testEnv.joinChannelHelper(alice, adminRoom, channel);

// Ensure we join the IRC side
await alice.sendText(cRoomId, `Hello world!`);
await bob.waitForEvent('message', 10000);

const quitEvent = bob.waitForEvent('quit', 10000);

// Now, have alice quit.
await alice.sendText(adminRoom, `!reconnect`);
const [nick, message, channels] = await quitEvent;
expect(nick).toEqual(`M-${opts.matrixLocalparts?.[0]}`);
expect(message).toEqual('Quit: Reconnecting');
expect(channels).toContain(channel);
});
});
11 changes: 7 additions & 4 deletions spec/util/e2e-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,9 @@ export class IrcBridgeE2ETest {
}
}),
}, registration);
return new IrcBridgeE2ETest(homeserver, ircBridge, registration, postgresDb, ircTest, redisPool, traceStream)
return new IrcBridgeE2ETest(
homeserver, ircBridge, registration, postgresDb, ircTest, opts, redisPool, traceStream
);
}

private constructor(
Expand All @@ -309,6 +311,7 @@ export class IrcBridgeE2ETest {
public readonly registration: AppServiceRegistration,
readonly postgresDb: string,
public readonly ircTest: TestIrcServer,
public readonly opts: Opts,
public readonly pool?: IrcConnectionPool,
private traceLog?: WriteStream,
) {
Expand Down Expand Up @@ -356,9 +359,6 @@ export class IrcBridgeE2ETest {
}

public async tearDown(): Promise<void> {
if (this.traceLog) {
this.traceLog.close();
}
await Promise.allSettled([
this.ircBridge?.kill(),
this.ircTest.tearDown(),
Expand All @@ -367,6 +367,9 @@ export class IrcBridgeE2ETest {
this.dropDatabase(),
]);
await this.pool?.close();
if (this.traceLog) {
this.traceLog.close();
}
}

public async createAdminRoomHelper(client: E2ETestMatrixClient): Promise<string> {
Expand Down
9 changes: 8 additions & 1 deletion src/pool-service/RedisIrcConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,18 @@ export type RedisIrcConnectionEvents = {

export class RedisIrcConnection extends (EventEmitter as unknown as
new () => TypedEmitter<RedisIrcConnectionEvents&IrcConnectionEventsMap>) implements IrcConnection {
private readonly log = new Logger(`RedisIrcConnection:${this.clientId}`);
private readonly log: Logger;

public get connecting() {
return this.isConnecting;
}

public get readyState(): string {
// TODO: Should this be just pulled directly from the socket.
// No support for readonly / writeonly.
return this.isConnecting ? 'opening' : 'open';
}

private isConnecting = true;
public localPort?: number;
public localIp?: string;
Expand All @@ -25,6 +31,7 @@ export class RedisIrcConnection extends (EventEmitter as unknown as
public readonly clientId: ClientId,
public state: IrcClientState) {
super();
this.log = new Logger(`RedisIrcConnection:${this.clientId}`);
this.once('connect', () => {
this.isConnecting = false;
});
Expand Down
Loading