From b5b1ebdfd27fe91e1a00aaf6165e7a0fc7709111 Mon Sep 17 00:00:00 2001 From: Aaron Bockelie Date: Fri, 15 May 2026 15:00:11 -0500 Subject: [PATCH] fix(dataview): serialize Luxon dates and propagate Dataview-internal errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #115. Closes #123. The Dataview API returns Luxon DateTime objects (which expose toISO(), not toISOString()) for file.ctime/mtime, so listPages() and getPageMetadata() crashed with "toISOString is not a function" the moment any real Dataview result reached them. The existing tests passed only because the mocks used native Date, which exposes toISOString(). The query path had a separate envelope bug: executeQuery() hard-coded success: true whenever dataviewAPI.query() didn't throw, while Dataview itself returns { successful: false, error: "..." } for malformed queries without throwing. The formatter then read response.successful (undefined), hit the failure branch, and rendered every result as "❌ Query failed: Unknown error" — even when the query had succeeded. - Add toIsoOptional() that prefers Luxon's toISO() and falls back to Date.toISOString(); apply at every page.file.ctime/mtime serialization site and inside convertDataviewValue(). - Propagate result.successful and result.error from the inner Dataview response to the outer tool envelope. - Add a dataview.query normalizer case in the formatter dispatcher that flattens result.{type,values,headers} to the top level and maps success → successful, matching the formatter's contract. - Regression tests for both bugs, including a Luxon-shaped mock distinct from the existing Date-shaped one. --- src/formatters/index.ts | 24 +++++++++ src/tools/dataview-tool.ts | 54 ++++++++++++++----- tests/dataview-integration.test.ts | 84 ++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 12 deletions(-) diff --git a/src/formatters/index.ts b/src/formatters/index.ts index c15efed..ed09b3e 100644 --- a/src/formatters/index.ts +++ b/src/formatters/index.ts @@ -339,6 +339,30 @@ function normalizeResponse(key: string, response: unknown): NormalizedResponse { return resp; } + // dataview.query: tool returns + // {success, query, format, result: {type, values, headers?} | {type:'unknown', data}, type, error?, ...} + // Formatter expects {query, type, values?, headers?, successful, error?} at top level. + // Flatten result.{type,values,headers} and rename success → successful so the + // formatter renders typed results and surfaces Dataview's own error messages. + case 'dataview.query': { + const dvResp = resp as { + success?: boolean; + query?: string; + error?: string; + type?: string; + result?: { type?: string; values?: unknown; headers?: unknown }; + }; + const inner = dvResp.result; + return { + ...resp, + successful: dvResp.success ?? true, + type: inner?.type ?? dvResp.type ?? 'list', + values: inner?.values, + headers: inner?.headers, + error: dvResp.error + }; + } + // edit.window: router returns {isError, content}, formatter expects {success, path} case 'edit.window': case 'edit.from_buffer': { diff --git a/src/tools/dataview-tool.ts b/src/tools/dataview-tool.ts index 5280ff3..8615839 100644 --- a/src/tools/dataview-tool.ts +++ b/src/tools/dataview-tool.ts @@ -14,9 +14,33 @@ interface DataviewArray { map(fn: (item: T) => U): DataviewArray; } -/** Dataview date/time value with ISO serialization */ +/** + * Dataview date/time value. Dataview emits Luxon DateTime objects, which + * expose `toISO()` (returns `string | null`). Some test fixtures pass native + * `Date` objects that only expose `toISOString()`. We accept either shape. + */ interface DataviewDateTime { - toISOString(): string; + toISO?(): string | null; + toISOString?(): string; +} + +/** + * Serialize a Dataview date value to ISO 8601, preferring Luxon's `toISO()` + * and falling back to native `Date.toISOString()`. Returns `undefined` for + * nullish input, invalid Luxon dates, or values that expose neither method — + * which lets `?.` propagate cleanly and keeps the response shape stable. + */ +function toIsoOptional(value: unknown): string | undefined { + if (value == null) return undefined; + const dt = value as DataviewDateTime; + if (typeof dt.toISO === 'function') { + const iso = dt.toISO(); + if (iso) return iso; + } + if (typeof dt.toISOString === 'function') { + return dt.toISOString(); + } + return undefined; } /** Dataview link value */ @@ -62,6 +86,7 @@ interface DataviewQueryResult { type: string; values?: DataviewArray; headers?: string[]; + error?: string; } /** Row within a table result */ @@ -150,14 +175,18 @@ export class DataviewTool { try { if (format === 'dql') { - // Execute DQL query + // Execute DQL query. Dataview returns {successful: false, error} + // for syntax/runtime errors rather than throwing — propagate that + // status to the outer envelope so the formatter can render it. const result: DataviewQueryResult = await dataviewAPI.query(query); + const innerSuccess = result.successful !== false; return { - success: true, + success: innerSuccess, query, format, result: this.formatQueryResult(result), type: result.type || 'unknown', + error: innerSuccess ? undefined : result.error, workflow: this.generateQueryWorkflow(query, result), hints: this.generateQueryHints(query) }; @@ -199,8 +228,8 @@ export class DataviewTool { path: page.file.path, name: page.file.name, size: page.file.size, - created: page.file.ctime?.toISOString(), - modified: page.file.mtime?.toISOString(), + created: toIsoOptional(page.file.ctime), + modified: toIsoOptional(page.file.mtime), tags: page.file.tags?.array() ?? [], links: page.file.outlinks?.array()?.length ?? 0, aliases: page.aliases?.array() ?? [], @@ -244,8 +273,8 @@ export class DataviewTool { basename: page.file.basename, extension: page.file.extension, size: page.file.size, - created: page.file.ctime?.toISOString(), - modified: page.file.mtime?.toISOString() + created: toIsoOptional(page.file.ctime), + modified: toIsoOptional(page.file.mtime) }, tags: page.file.tags?.array() ?? [], aliases: page.aliases?.array() ?? [], @@ -387,10 +416,11 @@ export class DataviewTool { return dvArray.array().map((item: unknown) => this.convertDataviewValue(item)); } - // Handle Dataview dates - const dvDate = value as { toISOString?: () => string }; - if (typeof dvDate.toISOString === 'function') { - return dvDate.toISOString(); + // Handle Dataview dates (Luxon DateTime exposes toISO(), native Date toISOString()) + const dvDate = value as DataviewDateTime; + if (typeof dvDate.toISO === 'function' || typeof dvDate.toISOString === 'function') { + const iso = toIsoOptional(value); + if (iso !== undefined) return iso; } // Handle Dataview links diff --git a/tests/dataview-integration.test.ts b/tests/dataview-integration.test.ts index b17a8b3..c518cbd 100644 --- a/tests/dataview-integration.test.ts +++ b/tests/dataview-integration.test.ts @@ -273,6 +273,90 @@ describe('Dataview Integration', () => { expect(result.metadata.custom.priority).toBe('high'); }); + test('should serialize Luxon DateTime values from listPages without crashing', async () => { + // Regression for #123 bug 2: Dataview emits Luxon DateTime objects + // (toISO() returns string|null) and not native Date.toISOString(). + const luxonDate = (iso: string) => ({ toISO: () => iso }); + const app = new MockApp(true, true); + const api = new MockObsidianAPI(app); + const tool = new DataviewTool(api as any); + + const dvApi = (app.plugins.plugins['dataview'] as any).api as MockDataviewAPI; + dvApi.pages = (() => ({ + length: 1, + array: () => [{ + file: { + path: 'Luxon.md', + name: 'Luxon.md', + size: 42, + ctime: luxonDate('2026-01-02T03:04:05.000Z'), + mtime: luxonDate('2026-01-02T03:04:06.000Z'), + tags: { array: () => [] }, + outlinks: { array: () => [] } + }, + aliases: { array: () => [] } + }] + })) as any; + + const result = tool.listPages() as any; + expect(result.success).toBe(true); + expect(result.pages[0].created).toBe('2026-01-02T03:04:05.000Z'); + expect(result.pages[0].modified).toBe('2026-01-02T03:04:06.000Z'); + }); + + test('should serialize Luxon DateTime values from getPageMetadata without crashing', async () => { + // Regression for #123 bug 3 + const luxonDate = (iso: string) => ({ toISO: () => iso }); + const app = new MockApp(true, true); + const api = new MockObsidianAPI(app); + const tool = new DataviewTool(api as any); + + const dvApi = (app.plugins.plugins['dataview'] as any).api as MockDataviewAPI; + dvApi.page = (path: string) => path === 'Luxon.md' ? { + file: { + path: 'Luxon.md', + name: 'Luxon.md', + basename: 'Luxon', + extension: 'md', + size: 42, + ctime: luxonDate('2026-01-02T03:04:05.000Z'), + mtime: luxonDate('2026-01-02T03:04:06.000Z'), + tags: { array: () => [] }, + outlinks: { array: () => [] }, + inlinks: { array: () => [] }, + tasks: { array: () => [] }, + lists: { array: () => [] } + }, + aliases: { array: () => [] } + } as any : null; + + const result = tool.getPageMetadata('Luxon.md') as any; + expect(result.success).toBe(true); + expect(result.metadata.file.created).toBe('2026-01-02T03:04:05.000Z'); + expect(result.metadata.file.modified).toBe('2026-01-02T03:04:06.000Z'); + }); + + test('should propagate Dataview-internal query failures to outer envelope', async () => { + // Regression for #115 / #123 bug 1: Dataview returns + // {successful: false, error: "..."} + // when a query is malformed instead of throwing. The tool used to + // hard-code success:true regardless, hiding the real error. + const app = new MockApp(true, true); + const api = new MockObsidianAPI(app); + const tool = new DataviewTool(api as any); + + const dvApi = (app.plugins.plugins['dataview'] as any).api as MockDataviewAPI; + dvApi.query = (() => ({ + successful: false, + type: 'table', + error: 'No field "nonexistent" on this page' + })) as any; + + const result = await tool.executeQuery('TABLE nonexistent FROM ""') as any; + expect(result.success).toBe(false); + expect(result.error).toBe('No field "nonexistent" on this page'); + }); + test('should validate queries', async () => { const app = new MockApp(true, true); const api = new MockObsidianAPI(app);