Skip to content
Open
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
12 changes: 11 additions & 1 deletion src/LiveQueryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const OP_TYPES = {
SUBSCRIBE: 'subscribe',
UNSUBSCRIBE: 'unsubscribe',
ERROR: 'error',
QUERY: 'query',
};

// The event we get back from LiveQuery server
Expand All @@ -34,6 +35,7 @@ const OP_EVENTS = {
ENTER: 'enter',
LEAVE: 'leave',
DELETE: 'delete',
RESULT: 'result',
};

// The event the LiveQuery client should emit
Expand All @@ -53,6 +55,7 @@ const SUBSCRIPTION_EMMITER_TYPES = {
ENTER: 'enter',
LEAVE: 'leave',
DELETE: 'delete',
RESULT: 'result',
};

// Exponentially-growing random delay
Expand Down Expand Up @@ -212,7 +215,7 @@ class LiveQueryClient {
subscribeRequest.sessionToken = sessionToken;
}

const subscription = new LiveQuerySubscription(this.requestId, query, sessionToken);
const subscription = new LiveQuerySubscription(this.requestId, query, sessionToken, this);
this.subscriptions.set(this.requestId, subscription);
this.requestId += 1;
this.connectPromise
Expand Down Expand Up @@ -425,6 +428,13 @@ class LiveQueryClient {
}
break;
}
case OP_EVENTS.RESULT: {
if (subscription) {
const objects = data.results.map(json => ParseObject.fromJSON(json, false));
subscription.emit(SUBSCRIPTION_EMMITER_TYPES.RESULT, objects);
}
break;
}
Comment on lines +431 to +437
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add null safety check for data.results.

The RESULT handler assumes data.results is always an array. If the server sends a malformed message without the results field, data.results.map() will throw a runtime error.

Consider adding a defensive check:

 case OP_EVENTS.RESULT: {
   if (subscription) {
+    if (!data.results || !Array.isArray(data.results)) {
+      break;
+    }
     const objects = data.results.map(json => ParseObject.fromJSON(json, false));
     subscription.emit(SUBSCRIPTION_EMMITER_TYPES.RESULT, objects);
   }
   break;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case OP_EVENTS.RESULT: {
if (subscription) {
const objects = data.results.map(json => ParseObject.fromJSON(json, false));
subscription.emit(SUBSCRIPTION_EMMITER_TYPES.RESULT, objects);
}
break;
}
case OP_EVENTS.RESULT: {
if (subscription) {
if (!data.results || !Array.isArray(data.results)) {
break;
}
const objects = data.results.map(json => ParseObject.fromJSON(json, false));
subscription.emit(SUBSCRIPTION_EMMITER_TYPES.RESULT, objects);
}
break;
}
🤖 Prompt for AI Agents
In src/LiveQueryClient.ts around lines 431 to 437, the RESULT branch assumes
data.results is always present and an array; add a defensive null/type check
before calling .map to avoid runtime errors when the server sends malformed
messages. Update the handler to verify data.results is an array (or provide an
empty array fallback) and only map when valid, then emit the subscription result
using the safe array; also consider logging or ignoring invalid payloads so
malformed messages don't throw.

default: {
// create, update, enter, leave, delete cases
if (!subscription) {
Expand Down
20 changes: 19 additions & 1 deletion src/LiveQuerySubscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,21 @@ class LiveQuerySubscription {
subscribePromise: any;
unsubscribePromise: any;
subscribed: boolean;
client: any;
emitter: EventEmitter;
on: EventEmitter['on'];
emit: EventEmitter['emit'];
/*
* @param {string | number} id - subscription id
* @param {string} query - query to subscribe to
* @param {string} sessionToken - optional session token
* @param {object} client - LiveQueryClient instance
*/
constructor(id: string | number, query: ParseQuery, sessionToken?: string) {
constructor(id: string | number, query: ParseQuery, sessionToken?: string, client?: any) {
this.id = id;
this.query = query;
this.sessionToken = sessionToken;
this.client = client;
this.subscribePromise = resolvingPromise();
this.unsubscribePromise = resolvingPromise();
this.subscribed = false;
Expand All @@ -130,6 +133,21 @@ class LiveQuerySubscription {
return liveQueryClient.unsubscribe(this);
});
}

/**
* Execute a query on this subscription.
* The results will be delivered via the 'result' event.
*/
find() {
if (this.client) {
this.client.connectPromise.then(() => {
this.client.socket.send(JSON.stringify({
op: 'query',
requestId: this.id,
}));
});
}
}
Comment on lines +141 to +150
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add error handling and consider user feedback for missing client.

The find() method has several potential issues:

  1. Silent failure when client is missing: If this.client is not set, the method does nothing without notifying the caller. This could lead to confusion.
  2. No error handling: If connectPromise rejects or socket.send() throws, errors are swallowed.
  3. No socket existence check: If this.client.socket is undefined, accessing send will throw.

Consider these improvements:

 find() {
   if (this.client) {
     this.client.connectPromise.then(() => {
-      this.client.socket.send(JSON.stringify({
-        op: 'query',
-        requestId: this.id,
-      }));
+      if (this.client.socket) {
+        this.client.socket.send(JSON.stringify({
+          op: 'query',
+          requestId: this.id,
+        }));
+      }
+    }).catch(error => {
+      this.emit('error', error);
     });
+  } else {
+    this.emit('error', new Error('Cannot execute find: client not available'));
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
find() {
if (this.client) {
this.client.connectPromise.then(() => {
this.client.socket.send(JSON.stringify({
op: 'query',
requestId: this.id,
}));
});
}
}
find() {
if (this.client) {
this.client.connectPromise.then(() => {
if (this.client.socket) {
this.client.socket.send(JSON.stringify({
op: 'query',
requestId: this.id,
}));
}
}).catch(error => {
this.emit('error', error);
});
} else {
this.emit('error', new Error('Cannot execute find: client not available'));
}
}
🤖 Prompt for AI Agents
In src/LiveQuerySubscription.ts around lines 141 to 150, the find() method
silently no-ops when this.client is missing, swallows connection/send errors,
and assumes this.client.socket exists; update it to explicitly handle the
missing-client case (either throw an Error or return/reject a Promise with a
clear message so callers get feedback), call
this.client.connectPromise.then(...).catch(...) to handle connection rejection,
and inside the then block check that this.client.socket is present before
calling socket.send (wrap send in try/catch and surface/log the error or reject
the returned Promise). Ensure the method returns a Promise (or consistently
signals failures) so callers can observe errors and add any contextual error
logging or event emission as appropriate.

}

export default LiveQuerySubscription;
93 changes: 92 additions & 1 deletion src/__tests__/LiveQueryClient-test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
jest.dontMock('../LiveQuerySubscription');
jest.dontMock('../LiveQueryClient');
jest.dontMock('../arrayContainsObject');
jest.dontMock('../canBeSerialized');
Expand All @@ -23,7 +24,6 @@ jest.dontMock('../UniqueInstanceStateController');
jest.dontMock('../unsavedChildren');
jest.dontMock('../ParseACL');
jest.dontMock('../ParseQuery');
jest.dontMock('../LiveQuerySubscription');
jest.dontMock('../LocalDatastore');
jest.dontMock('../WebSocketController');

Expand All @@ -38,6 +38,7 @@ jest.setMock('../LocalDatastore', mockLocalDatastore);
const CoreManager = require('../CoreManager').default;
const EventEmitter = require('../EventEmitter').default;
const LiveQueryClient = require('../LiveQueryClient').default;
const LiveQuerySubscription = require('../LiveQuerySubscription').default;
const ParseObject = require('../ParseObject').default;
const ParseQuery = require('../ParseQuery').default;
const WebSocketController = require('../WebSocketController').default;
Expand Down Expand Up @@ -1091,4 +1092,94 @@ describe('LiveQueryClient', () => {
const subscription = liveQueryClient.subscribe();
expect(subscription).toBe(undefined);
});

it('can handle WebSocket result response message', () => {
const liveQueryClient = new LiveQueryClient({
applicationId: 'applicationId',
serverURL: 'ws://test',
javascriptKey: 'javascriptKey',
masterKey: 'masterKey',
sessionToken: 'sessionToken',
});
// Add mock subscription
const subscription = new events.EventEmitter();
liveQueryClient.subscriptions.set(1, subscription);
const object1 = new ParseObject('Test');
object1.set('key', 'value1');
const object2 = new ParseObject('Test');
object2.set('key', 'value2');
const data = {
op: 'result',
clientId: 1,
requestId: 1,
results: [object1._toFullJSON(), object2._toFullJSON()],
};
const event = {
data: JSON.stringify(data),
};
// Register checked in advance
let isChecked = false;
subscription.on('result', function (objects) {
isChecked = true;
expect(objects.length).toBe(2);
expect(objects[0].get('key')).toEqual('value1');
expect(objects[1].get('key')).toEqual('value2');
});

liveQueryClient._handleWebSocketMessage(event);

expect(isChecked).toBe(true);
});

it('LiveQuerySubscription class has find method', () => {
expect(typeof LiveQuerySubscription.prototype.find).toBe('function');
});

it('subscription has find method', () => {
const liveQueryClient = new LiveQueryClient({
applicationId: 'applicationId',
serverURL: 'ws://test',
javascriptKey: 'javascriptKey',
masterKey: 'masterKey',
sessionToken: 'sessionToken',
});
const query = new ParseQuery('Test');
query.equalTo('key', 'value');

const subscription = liveQueryClient.subscribe(query);

expect(subscription).toBeInstanceOf(LiveQuerySubscription);
expect(typeof subscription.find).toBe('function');
});

it('can send query message via subscription', async () => {
const liveQueryClient = new LiveQueryClient({
applicationId: 'applicationId',
serverURL: 'ws://test',
javascriptKey: 'javascriptKey',
masterKey: 'masterKey',
sessionToken: 'sessionToken',
});
liveQueryClient.socket = {
send: jest.fn(),
};
const query = new ParseQuery('Test');
query.equalTo('key', 'value');

const subscription = liveQueryClient.subscribe(query);
liveQueryClient.connectPromise.resolve();
await liveQueryClient.connectPromise;

subscription.find();

// Need to wait for the sendMessage promise to resolve
await Promise.resolve();

const messageStr = liveQueryClient.socket.send.mock.calls[1][0];
const message = JSON.parse(messageStr);
expect(message).toEqual({
op: 'query',
requestId: 1,
});
});
});
8 changes: 7 additions & 1 deletion types/LiveQuerySubscription.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,21 @@ declare class LiveQuerySubscription {
subscribePromise: any;
unsubscribePromise: any;
subscribed: boolean;
client: any;
emitter: EventEmitter;
on: EventEmitter['on'];
emit: EventEmitter['emit'];
constructor(id: string | number, query: ParseQuery, sessionToken?: string);
constructor(id: string | number, query: ParseQuery, sessionToken?: string, client?: any);
/**
* Close the subscription
*
* @returns {Promise}
*/
unsubscribe(): Promise<void>;
/**
* Execute a query on this subscription.
* The results will be delivered via the 'result' event.
*/
find(): void;
}
export default LiveQuerySubscription;