From 35001e20d4c8fc339e719e70fce0d90f8f483ce0 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Mon, 9 Sep 2024 11:49:46 +0100 Subject: [PATCH 01/11] vault: Add some examples that show error messages from filter by function --- .../Functions/Custom Filters - Demo.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md b/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md index f6c5d5b7ef..3251885794 100644 --- a/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md +++ b/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md @@ -43,3 +43,31 @@ filename includes Custom Filters - Demo # Infer tag from heading filter by function task.heading.includes('#context/home') || task.tags.find( (tag) => tag === '#context/home' ) && true || false ``` + +## Demonstrate Error Handling + +### Parsing Errors + +This section demonstrate how Tasks handles errors when reading `filter by function` instructions. + +#### SyntaxError + +```tasks +filter by function task.due.formatAsDate( +``` + +### Evaluation Errors + +This section demonstrate how Tasks handles when evaluating `filter by function` instructions during searches. + +#### ReferenceError + +```tasks +filter by function hello +``` + +#### Non-existent task field + +```tasks +filter by function task.nonExistentField +``` From 64be81e58db932e5f679f289d866f1f3a5fbf39b Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Mon, 9 Sep 2024 12:41:59 +0100 Subject: [PATCH 02/11] test: Wrap lines in test to visualise behaviour --- tests/Query/Query.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/Query/Query.test.ts b/tests/Query/Query.test.ts index 74fa93a035..7881af7e26 100644 --- a/tests/Query/Query.test.ts +++ b/tests/Query/Query.test.ts @@ -1553,10 +1553,14 @@ describe('Query', () => { // Assert expect(queryResult.searchErrorMessage).toEqual( - 'Error: Search failed.\nThe error message was:\n "ReferenceError: wibble is not defined"', + `Error: Search failed. +The error message was: + "ReferenceError: wibble is not defined"`, ); expect(queryResultUpper.searchErrorMessage).toEqual( - 'Error: Search failed.\nThe error message was:\n "ReferenceError: WIBBLE is not defined"', + `Error: Search failed. +The error message was: + "ReferenceError: WIBBLE is not defined"`, ); }); }); From 50d904c61ba106e3c80fde4ff49bb3727509a1c7 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Mon, 9 Sep 2024 05:50:57 -0700 Subject: [PATCH 03/11] fix: Make 'filter by function' include the instruction in error messages --- src/Query/Query.ts | 17 ++++++++++++++++- tests/Query/Query.test.ts | 8 ++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Query/Query.ts b/src/Query/Query.ts index b3ff5c2799..c373e6a1d8 100644 --- a/src/Query/Query.ts +++ b/src/Query/Query.ts @@ -265,10 +265,16 @@ ${statement.explainStatement(' ')} this.debug(`Executing query: ${this.formatQueryForLogging()}`); const searchInfo = new SearchInfo(this.tasksFile, tasks); + + // Custom filter (filter by function) does not report the instruction line in any exceptions, + // for performance reasons. So we keep track of it here. + let mayHaveThrownOnThisInstruction: string | undefined = undefined; try { this.filters.forEach((filter) => { + mayHaveThrownOnThisInstruction = filter.instruction; tasks = tasks.filter((task) => filter.filterFunction(task, searchInfo)); }); + mayHaveThrownOnThisInstruction = undefined; const { debugSettings } = getSettings(); const tasksSorted = debugSettings.ignoreSortInstructions ? tasks : Sort.by(this.sorting, tasks, searchInfo); @@ -283,7 +289,16 @@ ${statement.explainStatement(' ')} return new QueryResult(taskGroups, tasksSorted.length); } catch (e) { const description = 'Search failed'; - return QueryResult.fromError(errorMessageForException(description, e)); + let rawErrorMessage = errorMessageForException(description, e); + + // if we know the filter that failed, and its text is not insider the error message, + // append it to aid user support. + if (mayHaveThrownOnThisInstruction && !rawErrorMessage.includes(mayHaveThrownOnThisInstruction)) { + rawErrorMessage += ` +Problem line: + "${mayHaveThrownOnThisInstruction}"`; + } + return QueryResult.fromError(rawErrorMessage); } } diff --git a/tests/Query/Query.test.ts b/tests/Query/Query.test.ts index 7881af7e26..9b2388f114 100644 --- a/tests/Query/Query.test.ts +++ b/tests/Query/Query.test.ts @@ -1555,12 +1555,16 @@ describe('Query', () => { expect(queryResult.searchErrorMessage).toEqual( `Error: Search failed. The error message was: - "ReferenceError: wibble is not defined"`, + "ReferenceError: wibble is not defined" +Problem line: + "${source}"`, ); expect(queryResultUpper.searchErrorMessage).toEqual( `Error: Search failed. The error message was: - "ReferenceError: WIBBLE is not defined"`, + "ReferenceError: WIBBLE is not defined" +Problem line: + "${source.toUpperCase()}"`, ); }); }); From 06178d47260308e3e34e679e9c86dfbced2131c6 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 18:46:12 -0700 Subject: [PATCH 04/11] vault: Easy checking of error messages on non-trivial lines BNy adding line continuations to invalid filters in 'Custom Filters - Demo.md' --- .../Tasks-Demo/Functions/Custom Filters - Demo.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md b/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md index 3251885794..ce08ca65b7 100644 --- a/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md +++ b/resources/sample_vaults/Tasks-Demo/Functions/Custom Filters - Demo.md @@ -53,7 +53,8 @@ This section demonstrate how Tasks handles errors when reading `filter by functi #### SyntaxError ```tasks -filter by function task.due.formatAsDate( +filter by function \ + task.due.formatAsDate( ``` ### Evaluation Errors @@ -63,11 +64,13 @@ This section demonstrate how Tasks handles when evaluating `filter by function` #### ReferenceError ```tasks -filter by function hello +filter by function \ + hello ``` #### Non-existent task field ```tasks -filter by function task.nonExistentField +filter by function \ + task.nonExistentField ``` From 2c002f119c6bb1c40db7a08d0896427cf134f2e7 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 18:47:02 -0700 Subject: [PATCH 05/11] comment: Fix typo in a comment --- src/Query/Query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Query/Query.ts b/src/Query/Query.ts index c373e6a1d8..2ae776c32a 100644 --- a/src/Query/Query.ts +++ b/src/Query/Query.ts @@ -291,7 +291,7 @@ ${statement.explainStatement(' ')} const description = 'Search failed'; let rawErrorMessage = errorMessageForException(description, e); - // if we know the filter that failed, and its text is not insider the error message, + // if we know the filter that failed, and its text is not inside the error message, // append it to aid user support. if (mayHaveThrownOnThisInstruction && !rawErrorMessage.includes(mayHaveThrownOnThisInstruction)) { rawErrorMessage += ` From 4f54a3a739fe363a3561a595df0eaae8d83a649d Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 18:51:20 -0700 Subject: [PATCH 06/11] refactor: . Extract method generateErrorMessage() --- src/Query/Query.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Query/Query.ts b/src/Query/Query.ts index 2ae776c32a..0b1674820d 100644 --- a/src/Query/Query.ts +++ b/src/Query/Query.ts @@ -246,6 +246,10 @@ ${source}`; } private setError(message: string, statement: Statement) { + this.generateErrorMessage(statement, message); + } + + private generateErrorMessage(statement: Statement, message: string) { if (statement.allLinesIdentical()) { this._error = `${message} Problem line: "${statement.rawInstruction}"`; From dbb8898678dc01c148c1ffdc3769ff1d86eae7fd Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 18:58:32 -0700 Subject: [PATCH 07/11] refactor: - Make method generateErrorMessage() static --- src/Query/Query.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Query/Query.ts b/src/Query/Query.ts index 0b1674820d..8c31cc8531 100644 --- a/src/Query/Query.ts +++ b/src/Query/Query.ts @@ -246,15 +246,15 @@ ${source}`; } private setError(message: string, statement: Statement) { - this.generateErrorMessage(statement, message); + this._error = Query.generateErrorMessage(statement, message); } - private generateErrorMessage(statement: Statement, message: string) { + private static generateErrorMessage(statement: Statement, message: string) { if (statement.allLinesIdentical()) { - this._error = `${message} + return `${message} Problem line: "${statement.rawInstruction}"`; } else { - this._error = `${message} + return `${message} Problem statement: ${statement.explainStatement(' ')} `; From 820a4318fe57c835b58cdf880e274eb6a6250951 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 19:07:04 -0700 Subject: [PATCH 08/11] test: Show error text from multi-line filters --- tests/Query/Query.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/Query/Query.test.ts b/tests/Query/Query.test.ts index 9b2388f114..b8c7e5b31d 100644 --- a/tests/Query/Query.test.ts +++ b/tests/Query/Query.test.ts @@ -1542,7 +1542,8 @@ describe('Query', () => { describe('error handling', () => { it('should catch an exception that occurs during searching', () => { // Arrange - const source = 'filter by function wibble'; + const source = `filter by function \\ +wibble`; const query = new Query(source); const queryUpper = new Query(source.toUpperCase()); const task = TaskBuilder.createFullyPopulatedTask(); @@ -1557,14 +1558,14 @@ describe('Query', () => { The error message was: "ReferenceError: wibble is not defined" Problem line: - "${source}"`, + "filter by function wibble"`, ); expect(queryResultUpper.searchErrorMessage).toEqual( `Error: Search failed. The error message was: "ReferenceError: WIBBLE is not defined" Problem line: - "${source.toUpperCase()}"`, + "FILTER BY FUNCTION WIBBLE"`, ); }); }); From 028b7085ee37fec8fc2311b0534d61642d091484 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 19:30:39 -0700 Subject: [PATCH 09/11] fix: Make 'filter by function' include the statement in error messages --- src/Query/Query.ts | 12 ++++-------- tests/Query/Query.test.ts | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Query/Query.ts b/src/Query/Query.ts index 8c31cc8531..b29defa7a6 100644 --- a/src/Query/Query.ts +++ b/src/Query/Query.ts @@ -272,10 +272,10 @@ ${statement.explainStatement(' ')} // Custom filter (filter by function) does not report the instruction line in any exceptions, // for performance reasons. So we keep track of it here. - let mayHaveThrownOnThisInstruction: string | undefined = undefined; + let mayHaveThrownOnThisInstruction: Statement | undefined = undefined; try { this.filters.forEach((filter) => { - mayHaveThrownOnThisInstruction = filter.instruction; + mayHaveThrownOnThisInstruction = filter.statement; tasks = tasks.filter((task) => filter.filterFunction(task, searchInfo)); }); mayHaveThrownOnThisInstruction = undefined; @@ -295,12 +295,8 @@ ${statement.explainStatement(' ')} const description = 'Search failed'; let rawErrorMessage = errorMessageForException(description, e); - // if we know the filter that failed, and its text is not inside the error message, - // append it to aid user support. - if (mayHaveThrownOnThisInstruction && !rawErrorMessage.includes(mayHaveThrownOnThisInstruction)) { - rawErrorMessage += ` -Problem line: - "${mayHaveThrownOnThisInstruction}"`; + if (mayHaveThrownOnThisInstruction) { + rawErrorMessage = Query.generateErrorMessage(mayHaveThrownOnThisInstruction, rawErrorMessage); } return QueryResult.fromError(rawErrorMessage); } diff --git a/tests/Query/Query.test.ts b/tests/Query/Query.test.ts index b8c7e5b31d..8e9551d2fe 100644 --- a/tests/Query/Query.test.ts +++ b/tests/Query/Query.test.ts @@ -1557,15 +1557,23 @@ wibble`; `Error: Search failed. The error message was: "ReferenceError: wibble is not defined" -Problem line: - "filter by function wibble"`, +Problem statement: + filter by function \\ + wibble + => + filter by function wibble +`, ); expect(queryResultUpper.searchErrorMessage).toEqual( `Error: Search failed. The error message was: "ReferenceError: WIBBLE is not defined" -Problem line: - "FILTER BY FUNCTION WIBBLE"`, +Problem statement: + FILTER BY FUNCTION \\ + WIBBLE + => + FILTER BY FUNCTION WIBBLE +`, ); }); }); From e5fe99ca3b7fc332a64680bd0b15290d75947c34 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 19:34:30 -0700 Subject: [PATCH 10/11] refactor: . Shorten variable name to possiblyBrokenStatement --- src/Query/Query.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Query/Query.ts b/src/Query/Query.ts index b29defa7a6..193b7c4d53 100644 --- a/src/Query/Query.ts +++ b/src/Query/Query.ts @@ -272,13 +272,13 @@ ${statement.explainStatement(' ')} // Custom filter (filter by function) does not report the instruction line in any exceptions, // for performance reasons. So we keep track of it here. - let mayHaveThrownOnThisInstruction: Statement | undefined = undefined; + let possiblyBrokenStatement: Statement | undefined = undefined; try { this.filters.forEach((filter) => { - mayHaveThrownOnThisInstruction = filter.statement; + possiblyBrokenStatement = filter.statement; tasks = tasks.filter((task) => filter.filterFunction(task, searchInfo)); }); - mayHaveThrownOnThisInstruction = undefined; + possiblyBrokenStatement = undefined; const { debugSettings } = getSettings(); const tasksSorted = debugSettings.ignoreSortInstructions ? tasks : Sort.by(this.sorting, tasks, searchInfo); @@ -295,8 +295,8 @@ ${statement.explainStatement(' ')} const description = 'Search failed'; let rawErrorMessage = errorMessageForException(description, e); - if (mayHaveThrownOnThisInstruction) { - rawErrorMessage = Query.generateErrorMessage(mayHaveThrownOnThisInstruction, rawErrorMessage); + if (possiblyBrokenStatement) { + rawErrorMessage = Query.generateErrorMessage(possiblyBrokenStatement, rawErrorMessage); } return QueryResult.fromError(rawErrorMessage); } From 94362770f5a2b35148894fdcaf636dbed9315197 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Tue, 10 Sep 2024 19:38:33 -0700 Subject: [PATCH 11/11] refactor: . Shorten variable name to message --- src/Query/Query.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Query/Query.ts b/src/Query/Query.ts index 193b7c4d53..ef1d41787a 100644 --- a/src/Query/Query.ts +++ b/src/Query/Query.ts @@ -293,12 +293,12 @@ ${statement.explainStatement(' ')} return new QueryResult(taskGroups, tasksSorted.length); } catch (e) { const description = 'Search failed'; - let rawErrorMessage = errorMessageForException(description, e); + let message = errorMessageForException(description, e); if (possiblyBrokenStatement) { - rawErrorMessage = Query.generateErrorMessage(possiblyBrokenStatement, rawErrorMessage); + message = Query.generateErrorMessage(possiblyBrokenStatement, message); } - return QueryResult.fromError(rawErrorMessage); + return QueryResult.fromError(message); } }