From e571d6e3bcbe4db998678cd9c5ef039a2a5022f7 Mon Sep 17 00:00:00 2001
From: Johannes Millan <johannes.millan@gmail.com>
Date: Fri, 2 Jan 2026 15:30:43 +0100
Subject: [PATCH] fix(error-handling): prevent [object Object] from appearing
 in error messages

Improve error text extraction utilities to never return "[object Object]"
when displaying error messages to users. The fix adds more robust fallback
mechanisms including:

- Check for message, name, statusText properties before calling toString()
- Detect "[object Object]" result and fallback to JSON.stringify()
- Provide meaningful fallback messages when all extraction methods fail

Fixes #5790
---
 .../error-handler-with-frontend-inform.ts     |  38 +++-
 src/app/imex/sync/get-sync-error-str.spec.ts  | 150 +++++++++++++++
 src/app/imex/sync/get-sync-error-str.ts       |  82 ++++++--
 src/app/util/get-error-text.spec.ts           | 178 ++++++++++++++++++
 src/app/util/get-error-text.ts                |  90 +++++++--
 5 files changed, 499 insertions(+), 39 deletions(-)
 create mode 100644 src/app/imex/sync/get-sync-error-str.spec.ts
 create mode 100644 src/app/util/get-error-text.spec.ts

diff --git a/electron/error-handler-with-frontend-inform.ts b/electron/error-handler-with-frontend-inform.ts
index d6eaaff9f..88780a0bb 100644
--- a/electron/error-handler-with-frontend-inform.ts
+++ b/electron/error-handler-with-frontend-inform.ts
@@ -56,20 +56,48 @@ function _handleError(
   }
 }
 
+const OBJECT_OBJECT_STR = '[object Object]';
+
 // eslint-disable-next-line prefer-arrow/prefer-arrow-functions
 function _getErrorStr(e: unknown): string {
   if (typeof e === 'string') {
     return e;
   }
+
+  if (e == null) {
+    return 'Unknown error';
+  }
+
+  // Check for message property first (standard Error and custom errors)
+  if (typeof (e as any).message === 'string' && (e as any).message) {
+    return (e as any).message;
+  }
+
   if (e instanceof Error) {
     return e.toString();
   }
-  if (typeof e === 'object' && e !== null) {
+
+  // Check for name property
+  if (typeof (e as any).name === 'string' && (e as any).name) {
+    return (e as any).name;
+  }
+
+  if (typeof e === 'object') {
     try {
-      return JSON.stringify(e);
-    } catch (err) {
-      return String(e);
+      const jsonStr = JSON.stringify(e);
+      if (jsonStr && jsonStr !== '{}') {
+        return jsonStr;
+      }
+    } catch {
+      // Circular reference - fall through
+    }
+
+    // Try toString but check for [object Object]
+    const str = String(e);
+    if (str && str !== OBJECT_OBJECT_STR) {
+      return str;
     }
   }
-  return String(e);
+
+  return 'Unknown error (unable to extract message)';
 }
diff --git a/src/app/imex/sync/get-sync-error-str.spec.ts b/src/app/imex/sync/get-sync-error-str.spec.ts
new file mode 100644
index 000000000..aea4739ae
--- /dev/null
+++ b/src/app/imex/sync/get-sync-error-str.spec.ts
@@ -0,0 +1,150 @@
+import { getSyncErrorStr } from './get-sync-error-str';
+import { HANDLED_ERROR_PROP_STR } from '../../app.constants';
+
+describe('getSyncErrorStr', () => {
+  it('should return string errors directly', () => {
+    expect(getSyncErrorStr('sync failed')).toBe('sync failed');
+  });
+
+  it('should handle null', () => {
+    expect(getSyncErrorStr(null)).toBe('Unknown sync error');
+  });
+
+  it('should handle undefined', () => {
+    expect(getSyncErrorStr(undefined)).toBe('Unknown sync error');
+  });
+
+  it('should extract message from Error instances', () => {
+    const err = new Error('sync error message');
+    expect(getSyncErrorStr(err)).toBe('sync error message');
+  });
+
+  it('should handle HANDLED_ERROR_PROP_STR', () => {
+    const err = { [HANDLED_ERROR_PROP_STR]: 'handled sync error' };
+    expect(getSyncErrorStr(err)).toBe('handled sync error');
+  });
+
+  it('should extract response.data string (Axios pattern)', () => {
+    const err = { response: { data: 'server error response' } };
+    expect(getSyncErrorStr(err)).toBe('server error response');
+  });
+
+  it('should extract response.data.message (nested API error)', () => {
+    const err = { response: { data: { message: 'API error message' } } };
+    expect(getSyncErrorStr(err)).toBe('API error message');
+  });
+
+  it('should extract name property', () => {
+    const err = { name: 'SyncError' };
+    expect(getSyncErrorStr(err)).toBe('SyncError');
+  });
+
+  it('should extract statusText for HTTP errors', () => {
+    const err = { statusText: 'Service Unavailable' };
+    expect(getSyncErrorStr(err)).toBe('Service Unavailable');
+  });
+
+  it('should never return [object Object]', () => {
+    const plainObject = { foo: 'bar' };
+    const result = getSyncErrorStr(plainObject);
+    expect(result).not.toBe('[object Object]');
+  });
+
+  it('should JSON.stringify objects without standard error properties', () => {
+    const err = { code: 'NETWORK_ERROR', retry: true };
+    const result = getSyncErrorStr(err);
+    expect(result).toContain('code');
+    expect(result).toContain('NETWORK_ERROR');
+  });
+
+  it('should handle empty objects', () => {
+    const result = getSyncErrorStr({});
+    expect(result).toBe('Unknown sync error (unable to extract message)');
+  });
+
+  it('should prioritize HANDLED_ERROR_PROP_STR over message', () => {
+    const err = {
+      [HANDLED_ERROR_PROP_STR]: 'handled',
+      message: 'regular message',
+    };
+    expect(getSyncErrorStr(err)).toBe('handled');
+  });
+
+  it('should truncate long error messages', () => {
+    const longMessage = 'x'.repeat(500);
+    const err = { message: longMessage };
+    const result = getSyncErrorStr(err);
+    expect(result.length).toBeLessThanOrEqual(403); // 400 + '...'
+  });
+
+  it('should handle objects with custom toString', () => {
+    const err = {
+      toString: () => 'custom sync error',
+    };
+    expect(getSyncErrorStr(err)).toBe('custom sync error');
+  });
+
+  it('should prefer message over response.data', () => {
+    const err = {
+      message: 'direct message',
+      response: { data: 'response data' },
+    };
+    expect(getSyncErrorStr(err)).toBe('direct message');
+  });
+
+  it('should handle circular reference objects gracefully', () => {
+    const err: any = { message: null, data: {} };
+    err.data.self = err; // circular reference
+    const result = getSyncErrorStr(err);
+    expect(result).not.toBe('[object Object]');
+    expect(result).toBe('Unknown sync error (unable to extract message)');
+  });
+
+  it('should handle arrays', () => {
+    const result = getSyncErrorStr(['sync error 1', 'sync error 2']);
+    expect(result).not.toBe('[object Object]');
+    expect(result).toContain('sync error');
+  });
+
+  it('should handle numbers', () => {
+    expect(getSyncErrorStr(503)).toBe('503');
+  });
+
+  it('should handle objects where toString throws', () => {
+    const err = {
+      toString: () => {
+        throw new Error('toString failed');
+      },
+    };
+    const result = getSyncErrorStr(err);
+    expect(result).not.toBe('[object Object]');
+  });
+
+  it('should handle empty string message', () => {
+    const err = { message: '' };
+    const result = getSyncErrorStr(err);
+    // Empty message falls through to JSON.stringify
+    expect(result).not.toBe('[object Object]');
+    expect(result).toContain('message');
+  });
+
+  it('should handle WebDAV-style errors', () => {
+    const err = {
+      status: 423,
+      statusText: 'Locked',
+      response: { data: 'Resource is locked by another process' },
+    };
+    expect(getSyncErrorStr(err)).toBe('Resource is locked by another process');
+  });
+
+  it('should handle Dropbox-style API errors', () => {
+    const err = {
+      error: {
+        error_summary: 'path/not_found/...',
+        error: { tag: 'path', path: { tag: 'not_found' } },
+      },
+    };
+    const result = getSyncErrorStr(err);
+    expect(result).not.toBe('[object Object]');
+  });
+});
diff --git a/src/app/imex/sync/get-sync-error-str.ts b/src/app/imex/sync/get-sync-error-str.ts
index df5c26317..16ed64654 100644
--- a/src/app/imex/sync/get-sync-error-str.ts
+++ b/src/app/imex/sync/get-sync-error-str.ts
@@ -1,28 +1,76 @@
 import { truncate } from '../../util/truncate';
 import { HANDLED_ERROR_PROP_STR } from '../../app.constants';
+import { isObject } from '../../util/is-object';
 
-// ugly little helper to make sure we get the most information out of it for the user
+const OBJECT_OBJECT_STR = '[object Object]';
+
+// Helper to extract error string from various error shapes
 export const getSyncErrorStr = (err: unknown): string => {
-  let errorAsString: string =
-    err && (err as any)?.toString ? (err as any).toString() : '???';
-
-  // Check if error has a message property (most Error objects do)
-  if (err && typeof (err as any)?.message === 'string') {
-    errorAsString = (err as any).message as string;
+  // Handle string errors directly
+  if (typeof err === 'string') {
+    return truncate(err, 400);
   }
 
-  if (err && typeof (err as any)?.response?.data === 'string') {
-    errorAsString = (err as any)?.response?.data as string;
+  // Handle null/undefined
+  if (err == null) {
+    return 'Unknown sync error';
   }
 
-  if (
-    errorAsString === '[object Object]' &&
-    err &&
-    (err as any)[HANDLED_ERROR_PROP_STR]
-  ) {
-    errorAsString = (err as any)[HANDLED_ERROR_PROP_STR] as string;
+  const errAny = err as any;
+
+  // Check for handled error marker first (highest priority)
+  if (errAny[HANDLED_ERROR_PROP_STR]) {
+    return truncate(String(errAny[HANDLED_ERROR_PROP_STR]), 400);
   }
 
-  // Increased from 150 to 400 to show more context, especially for HTTP errors
-  return truncate(errorAsString.toString(), 400);
+  // Check message property (standard Error objects)
+  if (typeof errAny.message === 'string' && errAny.message) {
+    return truncate(errAny.message, 400);
+  }
+
+  // Check response.data (Axios-style errors)
+  if (typeof errAny.response?.data === 'string' && errAny.response.data) {
+    return truncate(errAny.response.data, 400);
+  }
+
+  // Check response.data.message (nested API error)
+  if (typeof errAny.response?.data?.message === 'string') {
+    return truncate(errAny.response.data.message, 400);
+  }
+
+  // Check for name property
+  if (typeof errAny.name === 'string' && errAny.name) {
+    return truncate(errAny.name, 400);
+  }
+
+  // Check for statusText (HTTP errors)
+  if (typeof errAny.statusText === 'string' && errAny.statusText) {
+    return truncate(errAny.statusText, 400);
+  }
+
+  // Try toString() but check for [object Object]
+  if (typeof errAny.toString === 'function') {
+    try {
+      const str = errAny.toString();
+      if (str && str !== OBJECT_OBJECT_STR) {
+        return truncate(str, 400);
+      }
+    } catch {
+      // toString() threw - fall through to JSON.stringify
+    }
+  }
+
+  // Try JSON.stringify as last resort
+  if (isObject(err)) {
+    try {
+      const jsonStr = JSON.stringify(err);
+      if (jsonStr && jsonStr !== '{}') {
+        return truncate(jsonStr, 400);
+      }
+    } catch {
+      // Circular reference or other JSON error - fall through
+    }
+  }
+
+  return 'Unknown sync error (unable to extract message)';
 };
diff --git a/src/app/util/get-error-text.spec.ts b/src/app/util/get-error-text.spec.ts
new file mode 100644
index 000000000..c3ee5ea80
--- /dev/null
+++ b/src/app/util/get-error-text.spec.ts
@@ -0,0 +1,178 @@
+import { getErrorTxt } from './get-error-text';
+import { HANDLED_ERROR_PROP_STR } from '../app.constants';
+
+describe('getErrorTxt', () => {
+  it('should return string errors directly', () => {
+    expect(getErrorTxt('simple error')).toBe('simple error');
+  });
+
+  it('should handle null', () => {
+    expect(getErrorTxt(null)).toBe('Unknown error (null/undefined)');
+  });
+
+  it('should handle undefined', () => {
+    expect(getErrorTxt(undefined)).toBe('Unknown error (null/undefined)');
+  });
+
+  it('should extract message from Error instances', () => {
+    const err = new Error('test error message');
+    expect(getErrorTxt(err)).toBe('test error message');
+  });
+
+  it('should extract message from plain objects with message property', () => {
+    expect(getErrorTxt({ message: 'object error' })).toBe('object error');
+  });
+
+  it('should handle HANDLED_ERROR_PROP_STR', () => {
+    const err = { [HANDLED_ERROR_PROP_STR]: 'handled error message' };
+    expect(getErrorTxt(err)).toBe('handled error message');
+  });
+
+  it('should extract nested error.message (HttpErrorResponse pattern)', () => {
+    const err = { error: { message: 'nested error message' } };
+    expect(getErrorTxt(err)).toBe('nested error message');
+  });
+
+  it('should extract error.name when message is not available', () => {
+    const err = { error: { name: 'ValidationError' } };
+    expect(getErrorTxt(err)).toBe('ValidationError');
+  });
+
+  it('should extract deeply nested error.error.message', () => {
+    const err = { error: { error: { message: 'deep nested message' } } };
+    expect(getErrorTxt(err)).toBe('deep nested message');
+  });
+
+  it('should extract name property when message is not available', () => {
+    const err = { name: 'CustomError' };
+    expect(getErrorTxt(err)).toBe('CustomError');
+  });
+
+  it('should extract statusText for HTTP errors', () => {
+    const err = { statusText: 'Not Found' };
+    expect(getErrorTxt(err)).toBe('Not Found');
+  });
+
+  it('should never return [object Object]', () => {
+    const plainObject = { foo: 'bar' };
+    const result = getErrorTxt(plainObject);
+    expect(result).not.toBe('[object Object]');
+    expect(result).toContain('foo');
+  });
+
+  it('should JSON.stringify objects without standard error properties', () => {
+    const err = { code: 500, details: 'server error' };
+    const result = getErrorTxt(err);
+    expect(result).toContain('code');
+    expect(result).toContain('500');
+  });
+
+  it('should handle empty objects', () => {
+    const result = getErrorTxt({});
+    expect(result).toBe('Unknown error (unable to extract message)');
+  });
+
+  it('should prioritize HANDLED_ERROR_PROP_STR over message', () => {
+    const err = {
+      [HANDLED_ERROR_PROP_STR]: 'handled',
+      message: 'regular message',
+    };
+    expect(getErrorTxt(err)).toBe('handled');
+  });
+
+  it('should handle TypeError instances', () => {
+    const err = new TypeError('Cannot read property of undefined');
+    expect(getErrorTxt(err)).toBe('Cannot read property of undefined');
+  });
+
+  it('should handle objects with custom toString that does not return [object Object]', () => {
+    const err = {
+      toString: () => 'custom error string',
+    };
+    expect(getErrorTxt(err)).toBe('custom error string');
+  });
+
+  it('should truncate long JSON strings', () => {
+    const longValue = 'x'.repeat(300);
+    const err = { data: longValue };
+    const result = getErrorTxt(err);
+    expect(result.length).toBeLessThanOrEqual(203); // 200 + '...'
+  });
+
+  it('should handle circular reference objects gracefully', () => {
+    const err: any = { message: null, data: {} };
+    err.data.self = err; // circular reference
+    const result = getErrorTxt(err);
+    // Should not throw and should not return [object Object]
+    expect(result).not.toBe('[object Object]');
+    expect(result).toBe('Unknown error (unable to extract message)');
+  });
+
+  it('should handle arrays', () => {
+    const result = getErrorTxt(['error1', 'error2']);
+    expect(result).not.toBe('[object Object]');
+    expect(result).toContain('error1');
+  });
+
+  it('should handle numbers', () => {
+    expect(getErrorTxt(404)).toBe('404');
+  });
+
+  it('should handle booleans', () => {
+    expect(getErrorTxt(false)).toBe('false');
+  });
+
+  it('should handle objects where toString throws', () => {
+    const err = {
+      toString: () => {
+        throw new Error('toString failed');
+      },
+    };
+    const result = getErrorTxt(err);
+    expect(result).not.toBe('[object Object]');
+  });
+
+  it('should handle empty string message', () => {
+    const err = { message: '' };
+    const result = getErrorTxt(err);
+    // Empty message falls through to JSON.stringify
+    expect(result).not.toBe('[object Object]');
+    expect(result).toContain('message');
+  });
+
+  it('should handle RangeError instances', () => {
+    const err = new RangeError('Maximum call stack size exceeded');
+    expect(getErrorTxt(err)).toBe('Maximum call stack size exceeded');
+  });
+
+  it('should handle SyntaxError instances', () => {
+    const err = new SyntaxError('Unexpected token');
+    expect(getErrorTxt(err)).toBe('Unexpected token');
+  });
+
+  it('should handle DOMException-like objects', () => {
+    const err = { name: 'NotFoundError', message: 'Node was not found' };
+    expect(getErrorTxt(err)).toBe('Node was not found');
+  });
+
+  it('should handle HTTP response error objects', () => {
+    const err = {
+      status: 500,
+      statusText: 'Internal Server Error',
+      error: { message: 'Database connection failed' },
+    };
+    expect(getErrorTxt(err)).toBe('Database connection failed');
+  });
+
+  it('should handle Axios-style error with response.data.error', () => {
+    const err = {
+      response: {
+        status: 401,
+        data: { error: 'Unauthorized access' },
+      },
+    };
+    const result = getErrorTxt(err);
+    // Should JSON.stringify since no direct message property
+    expect(result).toContain('Unauthorized');
+  });
+});
diff --git a/src/app/util/get-error-text.ts b/src/app/util/get-error-text.ts
index f380f38b7..53debf543 100644
--- a/src/app/util/get-error-text.ts
+++ b/src/app/util/get-error-text.ts
@@ -1,24 +1,80 @@
 import { isObject } from './is-object';
 import { HANDLED_ERROR_PROP_STR } from '../app.constants';
 
+const OBJECT_OBJECT_STR = '[object Object]';
+
 export const getErrorTxt = (err: unknown): string => {
-  if (err && isObject((err as any).error)) {
-    return (
-      (err as any).error.message ||
-      (err as any).error.name ||
-      // for ngx translate...
-      (isObject((err as any).error.error)
-        ? (err as any).error.error.toString()
-        : (err as any).error) ||
-      (err as any).error
-    );
-  } else if (err && (err as any)[HANDLED_ERROR_PROP_STR]) {
-    return (err as any)[HANDLED_ERROR_PROP_STR];
-  } else if (err && (err as any).toString) {
-    return (err as any).toString();
-  } else if (typeof err === 'string') {
+  // Handle string errors directly
+  if (typeof err === 'string') {
     return err;
-  } else {
-    return 'Unknown getErrorTxt error';
   }
+
+  // Handle null/undefined
+  if (err == null) {
+    return 'Unknown error (null/undefined)';
+  }
+
+  const errAny = err as any;
+
+  // Check for handled error marker first
+  if (errAny[HANDLED_ERROR_PROP_STR]) {
+    return errAny[HANDLED_ERROR_PROP_STR];
+  }
+
+  // Check direct message property (standard Error objects)
+  if (typeof errAny.message === 'string' && errAny.message) {
+    return errAny.message;
+  }
+
+  // Check nested error.message (HttpErrorResponse pattern)
+  if (isObject(errAny.error)) {
+    if (typeof errAny.error.message === 'string' && errAny.error.message) {
+      return errAny.error.message;
+    }
+    if (typeof errAny.error.name === 'string' && errAny.error.name) {
+      return errAny.error.name;
+    }
+    // Handle deeper nesting (ngx-translate pattern)
+    if (isObject(errAny.error.error)) {
+      if (typeof errAny.error.error.message === 'string') {
+        return errAny.error.error.message;
+      }
+    }
+  }
+
+  // Check for name property (some Error subclasses)
+  if (typeof errAny.name === 'string' && errAny.name) {
+    return errAny.name;
+  }
+
+  // Check for statusText (HTTP errors)
+  if (typeof errAny.statusText === 'string' && errAny.statusText) {
+    return errAny.statusText;
+  }
+
+  // Try toString() but check for [object Object]
+  if (typeof errAny.toString === 'function') {
+    try {
+      const str = errAny.toString();
+      if (str && str !== OBJECT_OBJECT_STR) {
+        return str;
+      }
+    } catch {
+      // toString() threw - fall through to JSON.stringify
+    }
+  }
+
+  // Try JSON.stringify as last resort for objects
+  if (isObject(err)) {
+    try {
+      const jsonStr = JSON.stringify(err);
+      if (jsonStr && jsonStr !== '{}') {
+        return jsonStr.length > 200 ? jsonStr.substring(0, 200) + '...' : jsonStr;
+      }
+    } catch {
+      // Circular reference or other JSON error - fall through
+    }
+  }
+
+  return 'Unknown error (unable to extract message)';
 };