Skip to content

Commit

Permalink
Add event handlers to redis client
Browse files Browse the repository at this point in the history
If certain event handlers aren't defined on the client before
connecting, the app will crash on disconnect event (e.g., timeout).

Issue comment regarding issue details from node-redis repo:
redis/node-redis#2032 (comment)

Fixes #129, #160
  • Loading branch information
Cool-Dev121 committed Apr 6, 2023
1 parent 33b520e commit 91ea708
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/sixty-suns-brush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/shopify-app-session-storage-redis': patch
---

Add event handlers to redis client to prevent crashing on disconnect event. Fixes #129, #160 (Thanks to @davidhollenbeckx for linking to issue and solution.)
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
user shopify on +@all allkeys >passify
timeout 2
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ describe('RedisSessionStorage', () => {
await wait(10000);

client = createClient({url: dbURL.toString()});
client.on('error', (err) => {});
client.on('connect', () => {});
client.on('reconnecting', () => {});
client.on('ready', () => {});
await client.connect();
});

Expand Down Expand Up @@ -161,6 +165,32 @@ describe('RedisSessionStorage', () => {
storageClone1.disconnect();
storageClone2.disconnect();
});

it(`reconnects after a timeout`, async () => {
const storage = new RedisSessionStorage(dbURL);
await storage.ready;

const sessionId = 'timeout_connection_test';
const session = new Session({
id: sessionId,
shop: 'shop1.myshopify.com',
state: 'state',
isOnline: false,
scope: ['test_scope'].toString(),
accessToken: 'abcde-12345-123',
});

await storage.storeSession(session);

// Wait for the redis client to disconnect
await wait(2500);

const storedSession = await storage.loadSession(sessionId);
expect(storedSession).toBeDefined();
expect(storedSession?.id).toBe(sessionId);

await storage.disconnect();
});
});

async function initWithNonSessionData(client: RedisClient) {
Expand Down
18 changes: 8 additions & 10 deletions packages/shopify-app-session-storage-redis/src/redis-connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,9 @@ export class RedisConnection implements DBConnection {
sessionStorageIdentifier: string;
private client: RedisClient;

constructor(
dbUrl: string,
options: RedisClientOptions,
keyPrefix: string,
onError?: (...args: any[]) => void,
) {
constructor(dbUrl: string, options: RedisClientOptions, keyPrefix: string) {
this.init(dbUrl, options);
this.sessionStorageIdentifier = keyPrefix;

if (onError) {
this.client.on('error', onError);
}
}

query(_query: string, _params: any[]): Promise<any[]> {
Expand Down Expand Up @@ -62,5 +53,12 @@ export class RedisConnection implements DBConnection {
...options,
url: dbUrl,
});

this.client.on('error', this.eventHandler);
this.client.on('connect', this.eventHandler);
this.client.on('reconnecting', this.eventHandler);
this.client.on('ready', this.eventHandler);
}

private eventHandler = (..._args: any[]) => {};
}

0 comments on commit 91ea708

Please sign in to comment.