(core) Improving error messages on file imports

Summary: Improving error messages that get returned from "Import from URL" plugin.

Test Plan: browser tests

Reviewers: alexmojaki

Reviewed By: alexmojaki

Subscribers: dsagal, alexmojaki

Differential Revision: https://phab.getgrist.com/D2946

app/server/lib/DocPluginManager.ts

@@ -1,18 +1,18 @@
-import {ApplyUAResult} from 'app/common/ActiveDocAPI';
+import { ApplyUAResult } from 'app/common/ActiveDocAPI';
-import {fromTableDataAction, TableColValues} from 'app/common/DocActions';
+import { fromTableDataAction, TableColValues } from 'app/common/DocActions';
 import * as gutil from 'app/common/gutil';
-import {LocalPlugin} from 'app/common/plugin';
+import { LocalPlugin } from 'app/common/plugin';
-import {createRpcLogger, PluginInstance} from 'app/common/PluginInstance';
+import { createRpcLogger, PluginInstance } from 'app/common/PluginInstance';
-import {Promisified} from 'app/common/tpromisified';
+import { Promisified } from 'app/common/tpromisified';
-import {ParseFileResult, ParseOptions} from 'app/plugin/FileParserAPI';
+import { ParseFileResult, ParseOptions } from 'app/plugin/FileParserAPI';
-import {checkers, GristTable} from "app/plugin/grist-plugin-api";
+import { checkers, GristTable } from "app/plugin/grist-plugin-api";
-import {GristDocAPI} from "app/plugin/GristAPI";
+import { GristDocAPI } from "app/plugin/GristAPI";
-import {Storage} from 'app/plugin/StorageAPI';
+import { Storage } from 'app/plugin/StorageAPI';
-import {ActiveDoc} from 'app/server/lib/ActiveDoc';
+import { ActiveDoc } from 'app/server/lib/ActiveDoc';
-import {DocPluginData} from 'app/server/lib/DocPluginData';
+import { DocPluginData } from 'app/server/lib/DocPluginData';
-import {makeExceptionalDocSession} from 'app/server/lib/DocSession';
+import { makeExceptionalDocSession } from 'app/server/lib/DocSession';
-import {FileParserElement} from 'app/server/lib/FileParserElement';
+import { FileParserElement } from 'app/server/lib/FileParserElement';
-import {GristServer} from 'app/server/lib/GristServer';
+import { GristServer } from 'app/server/lib/GristServer';
 import * as log from 'app/server/lib/log';
 import { SafePythonComponent } from 'app/server/lib/SafePythonComponent';
 import { UnsafeNodeComponent } from 'app/server/lib/UnsafeNodeComponent';
@@ -28,14 +28,14 @@ promisifyAll(tmp);
  * Implements GristDocAPI interface.
  */
 class GristDocAPIImpl implements GristDocAPI {
-  constructor(private _activeDoc: ActiveDoc) {}
+  constructor(private _activeDoc: ActiveDoc) { }
 
   public async getDocName() { return this._activeDoc.docName; }
 
   public async listTables(): Promise<string[]> {
     const table = this._activeDoc.docData!.getTable('_grist_Tables')!;
     return (table.getColValues('tableId') as string[])
-    .filter(id => !id.startsWith("GristSummary_")).sort();
+      .filter(id => !id.startsWith("GristSummary_")).sort();
   }
 
   public async fetchTable(tableId: string): Promise<TableColValues> {
@@ -57,7 +57,7 @@ class GristDocAPIImpl implements GristDocAPI {
  */
 export class DocPluginManager {
 
-  public readonly plugins: {[s: string]: PluginInstance} = {};
+  public readonly plugins: { [s: string]: PluginInstance } = {};
   public readonly ready: Promise<any>;
   public readonly gristDocAPI: GristDocAPI;
 
@@ -101,6 +101,10 @@ export class DocPluginManager {
       }
     }
 
+    if (path.extname(fileName) === '.grist') {
+      throw new Error(`To import a grist document use the "Import document" menu option on your home screen`);
+    }
+
     const matchingFileParsers: FileParserElement[] = FileParserElement.getMatching(this._pluginInstances, fileName);
 
     if (!this._tmpDir) {
@@ -113,11 +117,11 @@ export class DocPluginManager {
     filePath = path.relative(this._tmpDir, filePath);
     log.debug(`parseFile: found ${matchingFileParsers.length} fileParser with matching file extensions`);
     const messages = [];
-    for (const {plugin, parseFileStub} of matchingFileParsers) {
+    for (const { plugin, parseFileStub } of matchingFileParsers) {
       const name = plugin.definition.id;
       try {
         log.info(`DocPluginManager.parseFile: calling to ${name} with ${filePath}`);
-        const result = await parseFileStub.parseFile({path: filePath, origName: fileName}, parseOptions);
+        const result = await parseFileStub.parseFile({ path: filePath, origName: fileName }, parseOptions);
         checkers.ParseFileResult.check(result);
         checkReferences(result.tables);
         return result;
@@ -128,8 +132,18 @@ export class DocPluginManager {
         continue;
       }
     }
-    const details = messages.length ? ": " + messages.join("; ") : "";
+
-    throw new Error(`Cannot parse this data${details}`);
+    if (messages.length) {
+      const extToType: Record<string, string> = {
+        '.xlsx' : 'Excel',
+        '.xls' : 'Excel',
+        '.json' : 'JSON',
+        '.csv' : 'CSV',
+      };
+      const fileType = extToType[path.extname(fileName)] || path.extname(fileName);
+      throw new Error(`Failed to parse ${fileType} file. Error: ${messages.join("; ")}`);
+    }
+    throw new Error(`File format is not supported.`);
   }
 
   /**
@@ -173,7 +187,7 @@ export class DocPluginManager {
   }
 
   private async _initialize(): Promise<void> {
-    this._tmpDir = await tmp.dirAsync({prefix: 'grist-tmp-', unsafeCleanup: true});
+    this._tmpDir = await tmp.dirAsync({ prefix: 'grist-tmp-', unsafeCleanup: true });
     for (const plugin of this._localPlugins) {
       try {
         // todo: once Comm has been replaced by grain-rpc, pluginInstance.rpc should forward '*' to client
@@ -184,7 +198,7 @@ export class DocPluginManager {
           new DocPluginData(this._activeDoc.docStorage, plugin.id), checkers.Storage);
         const components = plugin.manifest.components;
         if (components) {
-          const {safePython, unsafeNode} = components;
+          const { safePython, unsafeNode } = components;
           if (safePython) {
             const comp = pluginInstance.safePython = new SafePythonComponent(plugin, safePython, this._tmpDir,
               this._activeDoc.docName, this._server);

app/server/lib/uploads.ts

@@ -337,36 +337,45 @@ export async function fetchURL(url: string, accessId: string|null): Promise<Uplo
  */
 async function _fetchURL(url: string, accessId: string|null, fileName: string,
                          headers?: {[key: string]: string}): Promise<UploadResult> {
-  const response: FetchResponse = await Deps.fetch(url, {
+  try {
-    redirect: 'follow',
+    const response: FetchResponse = await Deps.fetch(url, {
-    follow: 10,
+      redirect: 'follow',
-    headers
+      follow: 10,
-  });
+      headers
-  await _checkForError(response);
+    });
-  if (fileName === '') {
+    await _checkForError(response);
-    const disposition = response.headers.get('content-disposition') || '';
+    if (fileName === '') {
-    fileName = contentDisposition.parse(disposition).parameters.filename || 'document.grist';
+      const disposition = response.headers.get('content-disposition') || '';
+      fileName = contentDisposition.parse(disposition).parameters.filename || 'document.grist';
+    }
+    const mimeType = response.headers.get('content-type');
+    const {tmpDir, cleanupCallback} = await createTmpDir({});
+    // Any name will do for the single file in tmpDir, but note that fileName may not be valid.
+    const destPath = path.join(tmpDir, 'upload-content');
+    await new Promise((resolve, reject) => {
+      const dest = fse.createWriteStream(destPath, {autoClose: true});
+      response.body.on('error', reject);
+      dest.on('error', reject);
+      dest.on('finish', resolve);
+      response.body.pipe(dest);
+    });
+    const uploadedFile: FileUploadInfo = {
+      absPath: path.resolve(destPath),
+      origName: fileName,
+      size: (await fse.stat(destPath)).size,
+      ext: await guessExt(destPath, fileName, mimeType),
+    };
+    log.debug(`done fetching url: ${url} to ${destPath}`);
+    const uploadId = globalUploadSet.registerUpload([uploadedFile], tmpDir, cleanupCallback, accessId);
+    return {uploadId, files: [pick(uploadedFile, ['origName', 'size', 'ext'])]};
+  } catch(err) {
+    if (err?.code === "EPROTO" || // https vs http error
+        err?.code === "ECONNREFUSED" || // server does not listen
+        err?.code === "ENOTFOUND") { // could not resolve domain
+      throw new ApiError(`Can't connect to the server. The URL seems to be invalid. Error code ${err.code}`, 400);
+    }
+    throw err;
   }
-  const mimeType = response.headers.get('content-type');
-  const {tmpDir, cleanupCallback} = await createTmpDir({});
-  // Any name will do for the single file in tmpDir, but note that fileName may not be valid.
-  const destPath = path.join(tmpDir, 'upload-content');
-  await new Promise((resolve, reject) => {
-    const dest = fse.createWriteStream(destPath, {autoClose: true});
-    response.body.on('error', reject);
-    dest.on('error', reject);
-    dest.on('finish', resolve);
-    response.body.pipe(dest);
-  });
-  const uploadedFile: FileUploadInfo = {
-    absPath: path.resolve(destPath),
-    origName: fileName,
-    size: (await fse.stat(destPath)).size,
-    ext: await guessExt(destPath, fileName, mimeType),
-  };
-  log.debug(`done fetching url: ${url} to ${destPath}`);
-  const uploadId = globalUploadSet.registerUpload([uploadedFile], tmpDir, cleanupCallback, accessId);
-  return {uploadId, files: [pick(uploadedFile, ['origName', 'size', 'ext'])]};
 }
 
 /**
@@ -392,9 +401,31 @@ async function fetchDoc(homeUrl: string, docId: string, req: Request, accessId:
 
 // Re-issue failures as exceptions.
 async function _checkForError(response: FetchResponse) {
-  if (response.ok) { return; }
+  if (response.status === 403) {
+    throw new ApiError("Access to this resource was denied.", response.status);
+  }
+  if (response.ok) {
+    const contentType = response.headers.get("content-type");
+    if (contentType?.startsWith("text/html")) {
+      // Probably we hit some login page
+      if (response.url.startsWith("https://accounts.google.com")) {
+        throw new ApiError("Importing directly from a Google Drive URL is not supported yet. " +
+        'Use the "Import from Google Drive" menu option instead.', 403);
+      } else {
+        throw new ApiError("Could not import the requested file, check if you have all required permissions.", 403);
+      }
+    }
+    return;
+   }
   const body = await response.json().catch(() => ({}));
-  throw new ApiError(body.error || response.statusText, response.status, body.details);
+  if (response.status === 404) {
+    throw new ApiError("File can't be found at the requested URL.", 404);
+  } else if (response.status >= 500 && response.status < 600) {
+    throw new ApiError(`Remote server returned an error (${body.error || response.statusText})`,
+      response.status, body.details);
+  } else {
+    throw new ApiError(body.error || response.statusText, response.status, body.details);
+  }
 }
 
 /**