Merge pull request #457 from 18alantom/error-report-fixes

fix: patch issues from error reports

fyo/core/converter.ts

@@ -37,6 +37,7 @@ export class Converter {
     schemaName: string,
     rawValueMap: RawValueMap | RawValueMap[]
   ): DocValueMap | DocValueMap[] {
+    rawValueMap ??= {};
     if (Array.isArray(rawValueMap)) {
       return rawValueMap.map((dv) => this.#toDocValueMap(schemaName, dv));
     } else {
@@ -48,6 +49,7 @@ export class Converter {
     schemaName: string,
     docValueMap: DocValueMap | DocValueMap[]
   ): RawValueMap | RawValueMap[] {
+    docValueMap ??= {};
     if (Array.isArray(docValueMap)) {
       return docValueMap.map((dv) => this.#toRawValueMap(schemaName, dv));
     } else {

fyo/model/doc.ts

@@ -17,6 +17,7 @@ import {
 import { getIsNullOrUndef, getMapFromList, getRandomString } from 'utils';
 import { markRaw } from 'vue';
 import { isPesa } from '../utils/index';
+import { getDbSyncError } from './errorHelpers';
 import {
   areDocValuesEqual,
   getMissingMandatoryMessage,
@@ -682,7 +683,12 @@ export class Doc extends Observable<DocValue | Doc[]> {
     await this._preSync();
 
     const validDict = this.getValidDict(false, true);
-    const data = await this.fyo.db.insert(this.schemaName, validDict);
+    let data: DocValueMap;
+    try {
+      data = await this.fyo.db.insert(this.schemaName, validDict);
+    } catch (err) {
+      throw await getDbSyncError(err as Error, this, this.fyo);
+    }
     await this._syncValues(data);
 
     this.fyo.telemetry.log(Verb.Created, this.schemaName);
@@ -695,7 +701,11 @@ export class Doc extends Observable<DocValue | Doc[]> {
     await this._preSync();
 
     const data = this.getValidDict(false, true);
+    try {
       await this.fyo.db.update(this.schemaName, data);
+    } catch (err) {
+      throw await getDbSyncError(err as Error, this, this.fyo);
+    }
     await this._syncValues(data);
 
     return this;

fyo/model/errorHelpers.ts

@@ -0,0 +1,170 @@
+import { Fyo } from 'fyo';
+import { DuplicateEntryError, NotFoundError } from 'fyo/utils/errors';
+import {
+  DynamicLinkField,
+  Field,
+  FieldTypeEnum,
+  TargetField,
+} from 'schemas/types';
+import { Doc } from './doc';
+
+type NotFoundDetails = { label: string; value: string };
+
+export async function getDbSyncError(
+  err: Error,
+  doc: Doc,
+  fyo: Fyo
+): Promise<Error> {
+  if (err.message.includes('UNIQUE constraint failed:')) {
+    return getDuplicateEntryError(err, doc);
+  }
+
+  if (err.message.includes('FOREIGN KEY constraint failed')) {
+    return getNotFoundError(err, doc, fyo);
+  }
+  return err;
+}
+
+function getDuplicateEntryError(
+  err: Error,
+  doc: Doc
+): Error | DuplicateEntryError {
+  const matches = err.message.match(/UNIQUE constraint failed:\s(\w+)\.(\w+)$/);
+  if (!matches) {
+    return err;
+  }
+
+  const schemaName = matches[1];
+  const fieldname = matches[2];
+  if (!schemaName || !fieldname) {
+    return err;
+  }
+
+  const duplicateEntryError = new DuplicateEntryError(err.message, false);
+  const validDict = doc.getValidDict(false, true);
+  duplicateEntryError.stack = err.stack;
+  duplicateEntryError.more = {
+    schemaName,
+    fieldname,
+    value: validDict[fieldname],
+  };
+
+  return duplicateEntryError;
+}
+
+async function getNotFoundError(
+  err: Error,
+  doc: Doc,
+  fyo: Fyo
+): Promise<NotFoundError> {
+  const notFoundError = new NotFoundError(fyo.t`Cannot perform operation.`);
+  notFoundError.stack = err.stack;
+  notFoundError.more.message = err.message;
+
+  const details = await getNotFoundDetails(doc, fyo);
+  if (!details) {
+    notFoundError.shouldStore = true;
+    return notFoundError;
+  }
+
+  notFoundError.shouldStore = false;
+  notFoundError.message = fyo.t`${details.label} value ${details.value} does not exist.`;
+  return notFoundError;
+}
+
+async function getNotFoundDetails(
+  doc: Doc,
+  fyo: Fyo
+): Promise<NotFoundDetails | null> {
+  /**
+   * Since 'FOREIGN KEY constraint failed' doesn't inform
+   * how the operation failed, all Link and DynamicLink fields
+   * must be checked for value existance so as to provide a
+   * decent error message.
+   */
+  for (const field of doc.schema.fields) {
+    const details = await getNotFoundDetailsIfDoesNotExists(field, doc, fyo);
+    if (details) {
+      return details;
+    }
+  }
+
+  return null;
+}
+
+async function getNotFoundDetailsIfDoesNotExists(
+  field: Field,
+  doc: Doc,
+  fyo: Fyo
+): Promise<NotFoundDetails | null> {
+  const value = doc.get(field.fieldname);
+  if (field.fieldtype === FieldTypeEnum.Link && value) {
+    return getNotFoundLinkDetails(field as TargetField, value as string, fyo);
+  }
+
+  if (field.fieldtype === FieldTypeEnum.DynamicLink && value) {
+    return getNotFoundDynamicLinkDetails(
+      field as DynamicLinkField,
+      value as string,
+      fyo,
+      doc
+    );
+  }
+
+  if (
+    field.fieldtype === FieldTypeEnum.Table &&
+    (value as Doc[] | undefined)?.length
+  ) {
+    return getNotFoundTableDetails(value as Doc[], fyo);
+  }
+
+  return null;
+}
+
+async function getNotFoundLinkDetails(
+  field: TargetField,
+  value: string,
+  fyo: Fyo
+): Promise<NotFoundDetails | null> {
+  const { target } = field;
+  const exists = await fyo.db.exists(target as string, value);
+  if (!exists) {
+    return { label: field.label, value };
+  }
+
+  return null;
+}
+
+async function getNotFoundDynamicLinkDetails(
+  field: DynamicLinkField,
+  value: string,
+  fyo: Fyo,
+  doc: Doc
+): Promise<NotFoundDetails | null> {
+  const { references } = field;
+  const target = doc.get(references);
+  if (!target) {
+    return null;
+  }
+
+  const exists = await fyo.db.exists(target as string, value);
+  if (!exists) {
+    return { label: field.label, value };
+  }
+
+  return null;
+}
+
+async function getNotFoundTableDetails(
+  value: Doc[],
+  fyo: Fyo
+): Promise<NotFoundDetails | null> {
+  for (const childDoc of value) {
+    const details = getNotFoundDetails(childDoc, fyo);
+    if (details) {
+      return details;
+    }
+  }
+
+  return null;
+}

fyo/utils/errors.ts

@@ -1,9 +1,14 @@
 export class BaseError extends Error {
+  more: Record<string, unknown> = {};
   message: string;
   statusCode: number;
   shouldStore: boolean;
 
-  constructor(statusCode: number, message: string, shouldStore: boolean = true) {
+  constructor(
+    statusCode: number,
+    message: string,
+    shouldStore: boolean = true
+  ) {
     super(message);
     this.name = 'BaseError';
     this.statusCode = statusCode;
@@ -96,7 +101,7 @@ export function getDbError(err: Error) {
     return CannotCommitError;
   }
 
-  if (err.message.includes('SQLITE_CONSTRAINT: UNIQUE constraint failed:')) {
+  if (err.message.includes('UNIQUE constraint failed:')) {
     return DuplicateEntryError;
   }
 

main.ts

@@ -4,7 +4,7 @@ import {
   app,
   BrowserWindow,
   BrowserWindowConstructorOptions,
-  protocol,
+  protocol
 } from 'electron';
 import Store from 'electron-store';
 import { autoUpdater } from 'electron-updater';
@@ -41,6 +41,13 @@ export class Main {
       autoUpdater.logger = console;
     }
 
+    // https://github.com/electron-userland/electron-builder/issues/4987
+    app.commandLine.appendSwitch('disable-http2');
+    autoUpdater.requestHeaders = {
+      'Cache-Control':
+        'no-store, no-cache, must-revalidate, proxy-revalidate, max-age=0',
+    };
+
     Store.initRenderer();
 
     this.registerListeners();

main/registerAutoUpdaterListeners.ts

@@ -1,9 +1,10 @@
-import { autoUpdater } from 'electron-updater';
+import { app, dialog } from 'electron';
+import { autoUpdater, UpdateInfo } from 'electron-updater';
 import { Main } from '../main';
 import { IPC_CHANNELS } from '../utils/messages';
 
 export default function registerAutoUpdaterListeners(main: Main) {
-  autoUpdater.autoDownload = true;
+  autoUpdater.autoDownload = false;
   autoUpdater.autoInstallOnAppQuit = true;
 
   autoUpdater.on('error', (error) => {
@@ -13,5 +14,34 @@ export default function registerAutoUpdaterListeners(main: Main) {
     }
 
     main.mainWindow!.webContents.send(IPC_CHANNELS.MAIN_PROCESS_ERROR, error);
+    dialog.showErrorBox(
+      'Update Error: ',
+      error == null ? 'unknown' : (error.stack || error).toString()
+    );
+  });
+
+  autoUpdater.on('update-available', async (info: UpdateInfo) => {
+    const currentVersion = app.getVersion();
+    const nextVersion = info.version;
+    const isCurrentBeta = currentVersion.includes('beta');
+    const isNextBeta = nextVersion.includes('beta');
+
+    let downloadUpdate = true;
+    if (!isCurrentBeta && isNextBeta) {
+      const option = await dialog.showMessageBox({
+        type: 'info',
+        title: `Update Frappe Books?`,
+        message: `Download version ${nextVersion}?`,
+        buttons: ['Yes', 'No'],
+      });
+
+      downloadUpdate = option.response === 0;
+    }
+
+    if (!downloadUpdate) {
+      return;
+    }
+
+    await autoUpdater.downloadUpdate();
   });
 }

main/registerIpcMainActionListeners.ts

@@ -1,5 +1,5 @@
 import { app, dialog, ipcMain } from 'electron';
-import { autoUpdater, UpdateInfo } from 'electron-updater';
+import { autoUpdater } from 'electron-updater';
 import fs from 'fs/promises';
 import path from 'path';
 import databaseManager from '../backend/database/manager';
@@ -15,40 +15,6 @@ import {
 } from './helpers';
 import { saveHtmlAsPdf } from './saveHtmlAsPdf';
 
-autoUpdater.autoDownload = false;
-
-autoUpdater.on('error', (error) => {
-  dialog.showErrorBox(
-    'Update Error: ',
-    error == null ? 'unknown' : (error.stack || error).toString()
-  );
-});
-
-autoUpdater.on('update-available', async (info: UpdateInfo) => {
-  const currentVersion = app.getVersion();
-  const nextVersion = info.version;
-  const isCurrentBeta = currentVersion.includes('beta');
-  const isNextBeta = nextVersion.includes('beta');
-
-  let downloadUpdate = true;
-  if (!isCurrentBeta && isNextBeta) {
-    const option = await dialog.showMessageBox({
-      type: 'info',
-      title: `Update Frappe Books?`,
-      message: `Download version ${nextVersion}?`,
-      buttons: ['Yes', 'No'],
-    });
-
-    downloadUpdate = option.response === 0;
-  }
-
-  if (!downloadUpdate) {
-    return;
-  }
-
-  await autoUpdater.downloadUpdate();
-});
-
 export default function registerIpcMainActionListeners(main: Main) {
   ipcMain.handle(IPC_ACTIONS.GET_OPEN_FILEPATH, async (event, options) => {
     return await dialog.showOpenDialog(main.mainWindow!, options);

models/baseModels/Invoice/Invoice.ts

@@ -152,7 +152,7 @@ export abstract class Invoice extends Transactional {
       }
 
       const tax = await this.getTax(item.tax!);
-      for (const { account, rate } of tax.details as TaxDetail[]) {
+      for (const { account, rate } of (tax.details ?? []) as TaxDetail[]) {
         taxes[account] ??= {
           account,
           rate,

models/helpers.ts

@@ -228,8 +228,9 @@ export async function getExchangeRate({
       }
     } catch (error) {
       console.error(error);
-      throw new Error(
-        `Could not fetch exchange rate for ${fromCurrency} -> ${toCurrency}`
+      throw new NotFoundError(
+        `Could not fetch exchange rate for ${fromCurrency} -> ${toCurrency}`,
+        false
       );
     }
   } else {

package.json

@@ -58,8 +58,8 @@
     "electron-builder": "^23.0.3",
     "electron-devtools-installer": "^3.2.0",
     "electron-notarize": "^1.1.1",
-    "electron-rebuild": "^3.2.7",
-    "electron-updater": "^4.3.9",
+    "electron-rebuild": "^3.2.9",
+    "electron-updater": "^5.2.1",
     "eslint": "^7.32.0",
     "eslint-config-prettier": "^8.3.0",
     "eslint-plugin-prettier": "^4.0.0",
@@ -74,9 +74,12 @@
     "tsconfig-paths": "^3.14.1",
     "tslib": "^2.3.1",
     "typescript": "^4.6.2",
-    "vue-cli-plugin-electron-builder": "^2.0.0",
+    "vue-cli-plugin-electron-builder": "https://github.com/nklayman/vue-cli-plugin-electron-builder#ebb9183f4913f927d4e4f4eb1fbab61a960f7a09",
     "webpack": "^5.66.0"
   },
+  "resolutions": {
+    "electron-builder": "^23.3.3"
+  },
   "prettier": {
     "semi": true,
     "singleQuote": true,

reports/AccountReport.ts

@@ -461,6 +461,10 @@ function setValueMapOnAccountTreeNodes(
   rangeGroupedMap: AccountNameValueMapMap
 ) {
   for (const name of rangeGroupedMap.keys()) {
+    if (!accountTree[name]) {
+      continue;
+    }
+
     const valueMap = rangeGroupedMap.get(name)!;
     accountTree[name].valueMap = valueMap;
     accountTree[name].prune = false;
@@ -547,7 +551,7 @@ function pruneAccountTree(accountTree: AccountTree) {
 
 function getPrunedChildren(children: AccountTreeNode[]): AccountTreeNode[] {
   return children.filter((child) => {
-    if (child.children) {
+    if (child.children?.length) {
       child.children = getPrunedChildren(child.children);
     }
 

src/components/Controls/AutoComplete.vue

@@ -153,8 +153,11 @@ export default {
         .map(({ item }) => item);
     },
     setSuggestion(suggestion) {
+      if (suggestion) {
         this.linkValue = suggestion.label;
         this.triggerChange(suggestion.value);
+      }
+
       this.toggleDropdown(false);
     },
     onFocus(e, toggleDropdown) {

src/components/Dropdown.vue

@@ -188,7 +188,7 @@ export default {
       return emptyMessage;
     },
     async selectItem(d) {
-      if (!d.action) {
+      if (!d?.action) {
         return;
       }
 

src/components/SearchBar.vue

@@ -149,7 +149,10 @@
           <p>↑↓ {{ t`Navigate` }}</p>
           <p>↩ {{ t`Select` }}</p>
           <p><span class="tracking-tighter">esc</span> {{ t`Close` }}</p>
-          <button class="flex items-center hover:text-gray-800" @click="openDocs">
+          <button
+            class="flex items-center hover:text-gray-800"
+            @click="openDocs"
+          >
             <feather-icon name="help-circle" class="w-4 h-4 mr-1" />
             {{ t`Help` }}
           </button>
@@ -284,7 +287,7 @@ export default {
     },
     open() {
       this.openModal = true;
-      this.searcher.updateKeywords();
+      this.searcher?.updateKeywords();
       nextTick(() => {
         this.$refs.input.focus();
       });
@@ -317,6 +320,10 @@ export default {
       ref.scrollIntoView({ block: 'nearest' });
     },
     getGroupFilterButtonClass(g) {
+      if (!this.searcher) {
+        return '';
+      }
+
       const isOn = this.searcher.filters.groupFilters[g];
       const color = this.groupColorMap[g];
       if (isOn) {
@@ -363,6 +370,10 @@ export default {
       }, {});
     },
     suggestions() {
+      if (!this.searcher) {
+        return [];
+      }
+
       const suggestions = this.searcher.search(this.inputValue);
       if (this.limit === -1) {
         return suggestions;

src/errorHandling.ts

@@ -26,11 +26,17 @@ async function reportError(errorLogObj: ErrorLog) {
     error_name: errorLogObj.name,
     message: errorLogObj.message,
     stack: errorLogObj.stack,
+    platform: fyo.store.platform,
+    version: fyo.store.appVersion,
+    language: fyo.store.language,
+    instance_id: fyo.store.instanceId,
+    open_count: fyo.store.openCount,
+    country_code: fyo.singles.SystemSettings?.countryCode,
     more: stringifyCircular(errorLogObj.more ?? {}),
   };
 
   if (fyo.store.isDevelopment) {
-    console.log(body);
+    console.log('reportError', body);
   }
 
   await ipcRenderer.invoke(IPC_ACTIONS.SEND_ERROR, JSON.stringify(body));
@@ -88,8 +94,11 @@ export async function handleErrorWithDialog(
   const errorMessage = getErrorMessage(error, doc);
   await handleError(false, error, { errorMessage, doc });
 
-  const name = error.name ?? t`Error`;
-  const options: MessageDialogOptions = { message: name, detail: errorMessage };
+  const label = getErrorLabel(error);
+  const options: MessageDialogOptions = {
+    message: label,
+    detail: errorMessage,
+  };
 
   if (reportError) {
     options.detail = truncate(options.detail, { length: 128 });
@@ -100,7 +109,7 @@ export async function handleErrorWithDialog(
           reportIssue(getErrorLogObject(error, { errorMessage }));
         },
       },
-      { label: t`OK`, action() {} },
+      { label: t`Cancel`, action() {} },
     ];
   }
 
@@ -190,3 +199,52 @@ export function reportIssue(errorLogObj?: ErrorLog) {
   const urlQuery = getIssueUrlQuery(errorLogObj);
   ipcRenderer.send(IPC_MESSAGES.OPEN_EXTERNAL, urlQuery);
 }
+
+function getErrorLabel(error: Error) {
+  const name = error.name;
+  if (!name) {
+    return t`Error`;
+  }
+
+  if (name === 'BaseError') {
+    return t`Error`;
+  }
+
+  if (name === 'ValidationError') {
+    return t`Validation Error`;
+  }
+
+  if (name === 'NotFoundError') {
+    return t`Not Found`;
+  }
+
+  if (name === 'ForbiddenError') {
+    return t`Forbidden Error`;
+  }
+
+  if (name === 'DuplicateEntryError') {
+    return t`Duplicate Entry`;
+  }
+
+  if (name === 'LinkValidationError') {
+    return t`Link Validation Error`;
+  }
+
+  if (name === 'MandatoryError') {
+    return t`Mandatory Error`;
+  }
+
+  if (name === 'DatabaseError') {
+    return t`Database Error`;
+  }
+
+  if (name === 'CannotCommitError') {
+    return t`Cannot Commit Error`;
+  }
+
+  if (name === 'NotImplemented') {
+    return t`Error`;
+  }
+
+  return t`Error`;
+}

src/pages/ListView/List.vue

@@ -156,6 +156,10 @@ export default defineComponent({
       this.pageEnd = end;
     },
     setUpdateListeners() {
+      if (!this.schemaName) {
+        return;
+      }
+
       const listener = () => {
         this.updateData();
       };

src/utils/index.ts

@@ -4,9 +4,14 @@
 import { t } from 'fyo';
 import { Doc } from 'fyo/model/doc';
 import { isPesa } from 'fyo/utils';
-import { DuplicateEntryError, LinkValidationError } from 'fyo/utils/errors';
+import {
+  BaseError,
+  DuplicateEntryError,
+  LinkValidationError
+} from 'fyo/utils/errors';
 import { Money } from 'pesa';
 import { Field, FieldType, FieldTypeEnum } from 'schemas/types';
+import { fyo } from 'src/initFyo';
 
 export function stringifyCircular(
   obj: unknown,
@@ -24,7 +29,8 @@ export function stringifyCircular(
     }
 
     if (cacheValue.includes(value)) {
-      const circularKey = cacheKey[cacheValue.indexOf(value)] || '{self}';
+      const circularKey: string =
+        cacheKey[cacheValue.indexOf(value)] || '{self}';
       return ignoreCircular ? undefined : `[Circular:${circularKey}]`;
     }
 
@@ -84,16 +90,23 @@ export function convertPesaValuesToFloat(obj: Record<string, unknown>) {
 }
 
 export function getErrorMessage(e: Error, doc?: Doc): string {
-  let errorMessage = e.message || t`An error occurred.`;
+  const errorMessage = e.message || t`An error occurred.`;
 
-  const { schemaName, name }: { schemaName?: string; name?: string } =
-    doc ?? {};
-  const canElaborate = !!(schemaName && name);
+  let { schemaName, name } = doc ?? {};
+  if (!doc) {
+    schemaName = (e as BaseError).more?.schemaName as string | undefined;
+    name = (e as BaseError).more?.value as string | undefined;
+  }
 
-  if (e instanceof LinkValidationError && canElaborate) {
-    errorMessage = t`${schemaName} ${name} is linked with existing records.`;
-  } else if (e instanceof DuplicateEntryError && canElaborate) {
-    errorMessage = t`${schemaName} ${name} already exists.`;
+  if (!schemaName || !name) {
+    return errorMessage;
+  }
+
+  const label = fyo.db.schemaMap[schemaName]?.label ?? schemaName;
+  if (e instanceof LinkValidationError) {
+    return t`${label} ${name} is linked with existing records.`;
+  } else if (e instanceof DuplicateEntryError) {
+    return t`${label} ${name} already exists.`;
   }
 
   return errorMessage;

src/utils/search.ts

@@ -418,7 +418,7 @@ export class Search {
     const totalChildKeywords = Object.values(this.searchables)
       .filter((s) => s.isChild)
       .map((s) => this.keywords[s.schemaName]?.length ?? 0)
-      .reduce((a, b) => a + b);
+      .reduce((a, b) => a + b, 0);
 
     if (totalChildKeywords > 2_000) {
       this.set('skipTables', true);
@@ -523,7 +523,12 @@ export class Search {
     keys.sort((a, b) => parseFloat(b) - parseFloat(a));
     const array: SearchItems = [];
     for (const key of keys) {
-      this._pushDocSearchItems(groupedKeywords[key], array, input);
+      const keywords = groupedKeywords[key];
+      if (!keywords?.length) {
+        continue;
+      }
+
+      this._pushDocSearchItems(keywords, array, input);
       if (key === '0') {
         this._pushNonDocSearchItems(array, input);
       }