fix: fix infinite load on error

now code won't wait for user to close error message
and will display the error in the json doc

fixed #85

src/parquet-document-provider.ts

@@ -26,12 +26,11 @@ export class ParquetTextDocumentContentProvider implements vscode.TextDocumentCo
 
   async provideTextDocumentContent(uri: vscode.Uri): Promise<string | undefined> {
     // already loaded?
-    const document = this._documents.get(uri) || (await (async _ => {
+    if (!this._documents.has(uri)) {
       const document = await ParquetDocument.create(uri, this._onDidChange);
       this._documents.set(uri, document);
-      return document;
-    })());
+    }
 
-    return document.value;
+    return this._documents.get(uri)?.value;
   }
 }

src/parquet-document.ts

@@ -1,7 +1,6 @@
 import * as os from 'os';
 import * as path from 'path';
 import * as vscode from 'vscode';
-import { assert } from 'console';
 import { promises } from 'fs';
 import { getLogger } from './logger';
 import { createParquetBackend } from './parquet-backend-factory';
@@ -34,26 +33,25 @@ export default class ParquetDocument implements vscode.Disposable {
     }
   }
 
-  public static async create(uri: vscode.Uri, emitter: vscode.EventEmitter<vscode.Uri>): Promise<ParquetDocument> {
-    try {
-      const parquet = new ParquetDocument(uri, emitter);
-      await parquet.populate();
-      return parquet;
-    } catch (error) {
-      const message = `while reading ${uri}: ${error}`;
-      getLogger().error(message);
-      await vscode.window.showErrorMessage(`${error}`);
-      throw Error(message);
-    }
+  public static async create(uri: vscode.Uri, emitter: vscode.EventEmitter<vscode.Uri>) {
+    const parquet = new ParquetDocument(uri, emitter);
+    await parquet.tryPopulate();
+    return parquet;
   }
 
   get value() {
     return this._lines.join(os.EOL) + os.EOL;
   }
 
-  private tryPopulate(uri: vscode.Uri) {
-    assert(uri.fsPath == this._parquetPath);
-    this.populate().catch(error => getLogger().warn(`failed to populate ${this._parquetPath}: ${error}`))
+  private async tryPopulate() {
+    try {
+      await this.populate();
+    } catch (error) {
+      const message = `while reading ${this._parquetPath}: ${error}`;
+      getLogger().error(message);
+      void vscode.window.showErrorMessage(message);
+      this._lines.push(JSON.stringify({error: message}));
+    }
   }
 
   private async populate() {

test/integration/parquet-editor-provider.test.ts

@@ -58,18 +58,18 @@ describe('ParquetEditorProvider', function () {
     ['small', 'parquet-tools'],
     ['small', 'parquets'],
     ['small', 'arrow'],
-    ['large', 'parquets']
+    ['large', 'parquets'],
+    ['version_2', 'arrow']
   ])('shows %p using %p', async function (name, backend) {
     const parquet = await getUri(`${name}.parquet`);
     testFile = await copyTo(`${name}-${backend}.parquet`, parquet);
-    const checkChanged = new Promise(resolve => {
+    const checkChanged = new Promise((resolve, reject) => {
       const callback = jest.fn(async function (editor?: vscode.TextEditor) {
         expect(editor?.document.fileName).toBe(`${testFile.fsPath}.as.json`);
         const expected = await readFile(`${name}.json`);
         expect(editor?.document.getText()).toEqual(expected);
-        resolve(callback);
       });
-      disposables.push(vscode.window.onDidChangeActiveTextEditor(callback));
+      disposables.push(vscode.window.onDidChangeActiveTextEditor(editor => callback(editor).then(_ => resolve(callback)).catch(reject)));
     });
     jest.mocked(settings.backend).mockReturnValue(backend);
     const document = await editorProvider.openCustomDocument(testFile);
@@ -89,23 +89,40 @@ describe('ParquetEditorProvider', function () {
       expect(document.getText()).toEqual(expected);
     });
 
-    const checkSmall = new Promise(resolve =>
-      disposables.push(workspace.onDidOpenTextDocument(async document => {
-        await checkChanged(document, 'small');
-        resolve(checkChanged);
-      })));
+    const checkSmall = new Promise((resolve, reject) =>
+      disposables.push(workspace.onDidOpenTextDocument(document =>
+        checkChanged(document, 'small').then(_ => resolve(checkChanged)).catch(reject)
+      ))
+    );
     await (await editorProvider.openCustomDocument(testFile)).open();
     await expect(checkSmall).resolves.toBeCalledWith(expect.anything(), 'small');
 
     // make sure checkSmall is not called again
     disposables.pop()?.dispose();
 
-    const checkLarge = new Promise(resolve => disposables.push(workspace.onDidChangeTextDocument(async event => {
-      await checkChanged(event.document, 'large');
-      resolve(checkChanged);
-    })));
+    const checkLarge = new Promise((resolve, reject) =>
+      disposables.push(workspace.onDidChangeTextDocument(event =>
+        checkChanged(event.document, 'large').then(_ => resolve(checkChanged)).catch(reject)
+      ))
+    );
     await fs.copy(await getUri('large.parquet'), testFile, { overwrite: true });
 
     await expect(checkLarge).resolves.toHaveBeenLastCalledWith(expect.anything(), 'large');
   });
+
+  test('handles error', async function () {
+    const parquet = await getUri(`version_2.parquet`);
+    testFile = await copyTo(`version_2-parquets.parquet`, parquet);
+    const checkChanged = new Promise((resolve, reject) => {
+      const callback = jest.fn(async function (editor?: vscode.TextEditor) {
+        expect(editor?.document.fileName).toBe(`${testFile.fsPath}.as.json`);
+        expect(editor?.document.getText()).toMatch(new RegExp(`{\\"error\\":\\"while reading ${testFile.fsPath.replace(/\\/g, '\\\\\\\\')}: .*\\"}`));
+      });
+      disposables.push(vscode.window.onDidChangeActiveTextEditor(editor => callback(editor).then(_ => resolve(callback)).catch(reject)));
+    });
+    jest.mocked(settings.backend).mockReturnValue('parquets');
+    const document = await editorProvider.openCustomDocument(testFile);
+    await document.open();
+    await expect(checkChanged).resolves.toBeCalled();
+  });
 });

test/unit/backend.test.ts

@@ -7,19 +7,18 @@ import { CancellationToken } from 'vscode';
 import { BackendNames } from '../../src/backend-name';
 import { createParquetBackend } from '../../src/parquet-backend-factory';
 import * as vscode from 'vscode';
+import { workspace } from './workspace';
 
 jest.setTimeout(60000);
 
-const rootDir = path.join(__dirname, '..', '..');
-
 describe.each(BackendNames)("%s backend tests", (backendName) => {
   const backend = createParquetBackend(backendName);
-  const workspace = path.join(rootDir, 'test', 'workspace');
 
   const testFiles = {
     'parquet-tools': [
       ['small', 'small'],
       ['large', 'large'],
+      ['version_2', 'version_2']
     ],
     'parquets': [
       ['small', 'small'],
@@ -28,6 +27,7 @@ describe.each(BackendNames)("%s backend tests", (backendName) => {
     'arrow': [
       ['small', 'small'],
       ['large', 'large.arrow'],
+      ['version_2', 'version_2']
     ]
   };
 

test/unit/parquets.test.ts

@@ -0,0 +1,13 @@
+import toArray from '@async-generators/to-array';
+import { describe, expect, test } from '@jest/globals';
+import * as path from 'path';
+import { ParquetsBackend } from "../../src/parquets-backend";
+import { workspace } from './workspace';
+
+describe("ParquetsBackend tests", () => {
+  const backend = new ParquetsBackend();
+
+  test("throws on invalid parquet version", async function () {
+    await expect(toArray(backend.toJson(path.join(workspace, 'version_2.parquet')))).rejects.toThrow('invalid parquet version');
+  });
+});

test/unit/workspace.ts

@@ -0,0 +1,4 @@
+import * as path from 'path';
+
+const rootDir = path.join(__dirname, '..', '..');
+export const workspace = path.join(rootDir, 'test', 'workspace');

test/workspace/.vscode/settings.json

@@ -1,4 +1,5 @@
 {
     "parquet-viewer.parquetToolsPath": "parquet-tools-1.12.0-SNAPSHOT.jar",
-    "parquet-viewer.logging.level": "debug"
+    "parquet-viewer.logging.level": "debug",
+    "parquet-viewer.logging.panel": true
 }

test/workspace/version_2.json

@@ -0,0 +1,3 @@
+{"a":1}
+{"a":2}
+{"a":3}