diff --git a/src_js/lbug.d.ts b/src_js/lbug.d.ts index ddd1d52..9a2217b 100644 --- a/src_js/lbug.d.ts +++ b/src_js/lbug.d.ts @@ -394,7 +394,9 @@ export class QueryResult { getNumTuples(): number; /** - * Get the next row. + * Get the next row. This method is asynchronous and must be awaited + * (`await result.getNext()`); in synchronous code, use `getNextSync()` + * instead. * @returns Promise that resolves to the next row or null if no more rows */ getNext(): Promise | null>; @@ -419,6 +421,12 @@ export class QueryResult { /** * Get all rows of the query result. + * + * WARNING: this method is asynchronous and must be awaited + * (`await result.getAll()`). Using it without `await` yields a pending + * Promise (which `JSON.stringify` renders as `{}`), not the rows; + * serializing that Promise throws an error pointing here. In synchronous + * code, use `getAllSync()` instead. * @returns Promise that resolves to all rows */ getAll(): Promise[]>; @@ -440,7 +448,8 @@ export class QueryResult { ): void; /** - * Get the column data types. + * Get the column data types. This method is asynchronous and must be + * awaited; in synchronous code, use `getColumnDataTypesSync()` instead. * @returns Promise that resolves to array of data type strings */ getColumnDataTypes(): Promise; @@ -452,7 +461,8 @@ export class QueryResult { getColumnDataTypesSync(): string[]; /** - * Get the column names. + * Get the column names. This method is asynchronous and must be awaited; + * in synchronous code, use `getColumnNamesSync()` instead. * @returns Promise that resolves to array of column names */ getColumnNames(): Promise; diff --git a/src_js/query_result.js b/src_js/query_result.js index 2cb0a25..e6d6772 100644 --- a/src_js/query_result.js +++ b/src_js/query_result.js @@ -2,6 +2,24 @@ const assert = require("assert"); +const GET_ALL_AWAIT_MESSAGE = + "QueryResult.getAll() returns a Promise and must be awaited " + + "(`await result.getAll()`). In synchronous code, use `result.getAllSync()` instead. " + + "Serializing the un-awaited Promise (e.g. via JSON.stringify, which renders it as '{}') " + + "is almost certainly a bug."; + +/** + * Promise returned by QueryResult.getAll(). Behaves exactly like a native + * Promise, except that serializing it (e.g. via JSON.stringify, which renders + * a plain pending Promise as '{}') throws a descriptive error instead of + * silently producing '{}' that looks like an empty result. + */ +class GetAllPromise extends Promise { + toJSON() { + throw new Error(GET_ALL_AWAIT_MESSAGE); + } +} + class QueryResult { /** * Internal constructor. Use `Connection.query` or `Connection.execute` @@ -51,7 +69,9 @@ class QueryResult { } /** - * Get the next row of the query result. + * Get the next row of the query result. This method is asynchronous and must + * be awaited (`await result.getNext()`); in synchronous code, use + * `getNextSync()` instead. * @returns {Promise} a promise that resolves to the next row of the query result. The promise is rejected if there is an error. */ getNext() { @@ -98,16 +118,26 @@ class QueryResult { /** * Get all rows of the query result. + * + * WARNING: this method is asynchronous and returns a Promise — it must be + * awaited (`await result.getAll()`). Using it without `await` yields a + * pending Promise (which `JSON.stringify` renders as `{}`), not the rows; + * serializing that Promise throws an error pointing here. In synchronous + * code, use `getAllSync()` instead. * @returns {Promise>} a promise that resolves to all rows of the query result. The promise is rejected if there is an error. */ - async getAll() { - this._checkClosed(); - this._queryResult.resetIterator(); - const result = []; - while (this.hasNext()) { - result.push(await this.getNext()); - } - return result; + getAll() { + return new GetAllPromise((resolve, reject) => { + (async () => { + this._checkClosed(); + this._queryResult.resetIterator(); + const result = []; + while (this.hasNext()) { + result.push(await this.getNext()); + } + return result; + })().then(resolve, reject); + }); } /** @@ -141,7 +171,9 @@ class QueryResult { } /** - * Get the data types of the columns of the query result. + * Get the data types of the columns of the query result. This method is + * asynchronous and must be awaited; in synchronous code, use + * `getColumnDataTypesSync()` instead. * @returns {Promise>} a promise that resolves to the data types of the columns of the query result. The promise is rejected if there is an error. */ getColumnDataTypes() { @@ -166,7 +198,9 @@ class QueryResult { } /** - * Get the names of the columns of the query result. + * Get the names of the columns of the query result. This method is + * asynchronous and must be awaited; in synchronous code, use + * `getColumnNamesSync()` instead. * @returns {Promise>} a promise that resolves to the names of the columns of the query result. The promise is rejected if there is an error. */ getColumnNames() { @@ -192,6 +226,8 @@ class QueryResult { /** * Get the query summary (compiling and execution time) of the query result. + * This method is asynchronous and must be awaited; in synchronous code, use + * `getQuerySummarySync()` instead. * @returns {Promise} a promise that resolves to the query summary of the query result. The promise is rejected if there is an error. */ getQuerySummary() { diff --git a/test/test_query_result.js b/test/test_query_result.js index 5f84b3f..8039f2b 100644 --- a/test/test_query_result.js +++ b/test/test_query_result.js @@ -133,6 +133,18 @@ describe("Get all", function () { const ids = tuples.map((tuple) => tuple["a.ID"]); assert.deepEqual(ids, PERSON_IDS); }); + + it("should fail loudly instead of silently returning {} when not awaited", async function () { + const queryResult = await conn.query( + "MATCH (a:person) RETURN a.ID ORDER BY a.ID" + ); + const pending = queryResult.getAll(); + assert.instanceOf(pending, Promise); + assert.throws(() => JSON.stringify(pending), /getAllSync/); + const tuples = await pending; + const ids = tuples.map((tuple) => tuple["a.ID"]); + assert.deepEqual(ids, PERSON_IDS); + }); }); describe("All", function () { diff --git a/test/test_sync_api.js b/test/test_sync_api.js index 3032003..cab8e14 100644 --- a/test/test_sync_api.js +++ b/test/test_sync_api.js @@ -92,6 +92,20 @@ describe("Query execution", function () { describe("Query result", function () { + it("should fail loudly when getAll() is used without await in sync code", async function () { + const queryResult = conn.querySync( + "MATCH (a:person) RETURN a.ID ORDER BY a.ID" + ); + const pending = queryResult.getAll(); + assert.instanceOf(pending, Promise); + assert.throws(() => JSON.stringify(pending), /getAllSync/); + const rows = await pending; + assert.equal(rows.length, PERSON_IDS.length); + for (let i = 0; i < PERSON_IDS.length; ++i) { + assert.equal(rows[i]["a.ID"], PERSON_IDS[i]); + } + }); + it("should get all rows of a query result synchronously", function () { const queryResult = conn.querySync( "MATCH (a:person) RETURN a.ID ORDER BY a.ID"