Address review feedback: simplify toError and add changeset

- Remove JSON.stringify fallback from toError, use the same unknown
  error messaging as stringifyError for all non-string/non-error values
- Add try/catch to protect against values that throw on string
  conversion (e.g. null-prototype objects, symbols)
- Fix no-op `void toError(err)` in DeleteEntityConfirmationDialog
- Fix `${err}` producing [object Object] in UrlReaderProcessor
- Fix double toError call in openStackSwift
- Update JSDoc to accurately describe the behavior
- Add tests for throwing toString and circular objects
- Add changeset for all refactored packages

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Made-with: Cursor
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Made-with: Cursor

.changeset/add-to-error-utility.md

@@ -2,4 +2,4 @@
 '@backstage/errors': minor
 ---
 
-A new `toError` utility function is now available for converting unknown values to `ErrorLike` objects. If the value is already error-like it is returned as-is. Strings are used directly as the error message, and other values are stringified with a fallback to JSON to avoid unhelpful messages like `[object Object]`. Non-error causes passed to `CustomErrorBase` are now converted and stored using `toError` rather than discarded.
+A new `toError` utility function is now available for converting unknown values to `ErrorLike` objects. If the value is already error-like it is returned as-is, strings are used directly as the error message, and all other values are wrapped as `unknown error '<stringified>'`. Non-error causes passed to `CustomErrorBase` are now converted and stored using `toError` rather than discarded.

.changeset/deep-ripe-rats.md

@@ -0,0 +1,30 @@
+---
+'@backstage/backend-app-api': patch
+'@backstage/backend-defaults': patch
+'@backstage/cli': patch
+'@backstage/cli-common': patch
+'@backstage/cli-module-migrate': patch
+'@backstage/cli-module-new': patch
+'@backstage/cli-node': patch
+'@backstage/config-loader': patch
+'@backstage/core-components': patch
+'@backstage/repo-tools': patch
+'@backstage/plugin-auth': patch
+'@backstage/plugin-auth-backend': patch
+'@backstage/plugin-catalog': patch
+'@backstage/plugin-catalog-backend': patch
+'@backstage/plugin-catalog-backend-module-incremental-ingestion': patch
+'@backstage/plugin-catalog-import': patch
+'@backstage/plugin-catalog-react': patch
+'@backstage/plugin-catalog-unprocessed-entities': patch
+'@backstage/plugin-devtools-backend': patch
+'@backstage/plugin-mcp-actions-backend': patch
+'@backstage/plugin-notifications-backend-module-slack': patch
+'@backstage/plugin-scaffolder-backend': patch
+'@backstage/plugin-scaffolder-backend-module-github': patch
+'@backstage/plugin-search-backend-node': patch
+'@backstage/plugin-techdocs-backend': patch
+'@backstage/plugin-techdocs-node': patch
+---
+
+Migrated from `assertError` to `toError` for error handling.

packages/errors/src/errors/CustomErrorBase.ts

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import { stringifyError } from '../serialization/error';
 import { toError } from './assertion';
 
 /**
@@ -42,13 +41,19 @@ export class CustomErrorBase extends Error {
   readonly cause?: Error | undefined;
 
   constructor(message?: string, cause?: Error | unknown) {
+    const causeError = cause !== undefined ? toError(cause) : undefined;
+
     let fullMessage = message;
-    if (cause !== undefined) {
-      const causeStr = stringifyError(cause);
+    if (causeError !== undefined) {
+      const causeStr = String(causeError);
+      const causeMsg =
+        causeStr !== '[object Object]'
+          ? causeStr
+          : `${causeError.name}: ${causeError.message}`;
       if (fullMessage) {
-        fullMessage += `; caused by ${causeStr}`;
+        fullMessage += `; caused by ${causeMsg}`;
       } else {
-        fullMessage = `caused by ${causeStr}`;
+        fullMessage = `caused by ${causeMsg}`;
       }
     }
 
@@ -63,6 +68,6 @@ export class CustomErrorBase extends Error {
       }
     }
 
-    this.cause = cause !== undefined ? toError(cause) : undefined;
+    this.cause = causeError;
   }
 }

packages/errors/src/errors/assertion.test.ts

@@ -125,17 +125,10 @@ describe('toError', () => {
     expect(toError(true).message).toBe("unknown error 'true'");
   });
 
-  it('should wrap plain objects using JSON when toString is unhelpful', () => {
-    expect(toError({ name: 'e' }).message).toBe(`unknown error '{"name":"e"}'`);
-    expect(toError({ message: '' }).message).toBe(
-      `unknown error '{"message":""}'`,
+  it('should wrap plain objects', () => {
+    expect(toError({ name: 'e' }).message).toBe(
+      "unknown error '[object Object]'",
     );
-    expect(toError({ code: 404, detail: 'missing' }).message).toBe(
-      `unknown error '{"code":404,"detail":"missing"}'`,
-    );
-  });
-
-  it('should fall back to [object Object] for empty plain objects', () => {
     expect(toError({}).message).toBe("unknown error '[object Object]'");
   });
 
@@ -152,10 +145,25 @@ describe('toError', () => {
   it('should handle symbols', () => {
     const result = toError(Symbol('test'));
     expect(result).toBeInstanceOf(Error);
-    expect(result.message).toBe("unknown error 'Symbol(test)'");
+    expect(result.message).toBe("unknown error of type 'symbol'");
   });
 
   it('should handle BigInt', () => {
     expect(toError(BigInt(42)).message).toBe("unknown error '42'");
   });
+
+  it('should not throw for objects with a throwing toString', () => {
+    const obj = Object.create(null);
+    const result = toError(obj);
+    expect(result).toBeInstanceOf(Error);
+    expect(result.message).toBe("unknown error of type 'object'");
+  });
+
+  it('should not throw for circular objects', () => {
+    const obj: { self?: unknown } = {};
+    obj.self = obj;
+    const result = toError(obj);
+    expect(result).toBeInstanceOf(Error);
+    expect(result.message).toBe("unknown error '[object Object]'");
+  });
 });

packages/errors/src/errors/assertion.ts

@@ -75,8 +75,10 @@ export function assertError(value: unknown): asserts value is ErrorLike {
 /**
  * Converts an unknown value to an {@link ErrorLike} object.
  *
- * If the value is already an {@link ErrorLike} object, it is returned as-is. Otherwise, a new
- * `Error` is created with the value stringified as the message.
+ * If the value is already an {@link ErrorLike} object, it is returned as-is.
+ * If the value is a string, a new `Error` is created with that string as the message.
+ * For all other values, a new `Error` is created with a message of the form
+ * `unknown error '<stringified>'`.
  *
  * @public
  * @param value - an unknown value
@@ -89,12 +91,9 @@ export function toError(value: unknown): ErrorLike {
   if (typeof value === 'string') {
     return new Error(value) as ErrorLike;
   }
-  const str = String(value);
-  if (str === '[object Object]') {
-    const json = JSON.stringify(value);
-    if (json !== '{}') {
-      return new Error(`unknown error '${json}'`) as ErrorLike;
-    }
+  try {
+    return new Error(`unknown error '${value}'`) as ErrorLike;
+  } catch {
+    return new Error(`unknown error of type '${typeof value}'`) as ErrorLike;
   }
-  return new Error(`unknown error '${str}'`) as ErrorLike;
 }

plugins/catalog-backend/src/processors/UrlReaderProcessor.ts

@@ -110,10 +110,8 @@ export class UrlReaderProcessor implements CatalogProcessor {
       emit(processingResult.refresh(`${location.type}:${location.target}`));
     } catch (error) {
       const err = toError(error);
-      const message = `Unable to read ${location.type}, ${err}`.substring(
-        0,
-        5000,
-      );
+      const message =
+        `Unable to read ${location.type}, ${err.message}`.substring(0, 5000);
       if (err.name === 'NotModifiedError' && cacheItem) {
         for (const parseResult of cacheItem.value) {
           emit(parseResult);

plugins/catalog-unprocessed-entities/src/components/DeleteEntityConfirmationDialog.tsx

@@ -19,7 +19,6 @@ import Dialog from '@material-ui/core/Dialog';
 import DialogActions from '@material-ui/core/DialogActions';
 import DialogTitle from '@material-ui/core/DialogTitle';
 import { useState } from 'react';
-import { toError } from '@backstage/errors';
 
 interface DeleteEntityConfirmationProps {
   open: boolean;
@@ -36,8 +35,8 @@ export function DeleteEntityConfirmationDialog(
     setBusy(true);
     try {
       onConfirm();
-    } catch (err) {
-      void toError(err);
+    } catch {
+      // ignored
     } finally {
       setBusy(false);
     }

plugins/techdocs-node/src/stages/publish/openStackSwift.ts

@@ -222,8 +222,9 @@ export class OpenStackSwiftPublish implements PublisherBase {
 
           resolve(techdocsMetadata);
         } catch (err) {
-          this.logger.error(toError(err).message);
-          reject(new Error(toError(err).message));
+          const error = toError(err);
+          this.logger.error(error.message);
+          reject(error);
         }
       } else {
         reject({