fix(core)(#191): promote errMessage helper; fix lossy NametagMinter stringification

core/errors.ts

@@ -985,3 +985,38 @@ export class SphereError extends Error {
 export function isSphereError(err: unknown): err is SphereError {
   return err instanceof SphereError;
 }
+
+/**
+ * Lossy-safe stringification of an unknown error value (issue #191).
+ *
+ * The inline pattern `err instanceof Error ? err.message : String(err)`
+ * collapses object-shaped errors to the default `Object.prototype.toString`
+ * output (`'[object Object]'`), masking aggregator response payloads,
+ * structured RPC errors, and any other non-Error throw value with useful
+ * field-level forensics. NametagMinter's testnet failure surface is the
+ * highest-visibility instance — operators saw `Submit failed: [object Object]`
+ * with no way to distinguish rate-limit / API-key / faucet-exhausted /
+ * validation-rejected outcomes.
+ *
+ * `errMessage` collapses to the same `Error.message` / `string` paths but
+ * falls back to `JSON.stringify(redactCause(err))` for everything else,
+ * routing through the W40 redaction layer so cryptographic-secret /
+ * untrusted-payload fields never leak even on this debug path. The final
+ * `String(err)` is the bottom of the stack for non-stringifiable values
+ * (cycles that survive `redactCause`, BigInts in the redacted view, ...).
+ *
+ * @example
+ *   errMessage(new Error('boom'))           // 'boom'
+ *   errMessage('boom')                       // 'boom'
+ *   errMessage({ status: 'BAD_REQUEST' })    // '{"status":"BAD_REQUEST"}'
+ *   errMessage({ signedTransferTxBytes: u8 }) // '{"signedTransferTxBytes":"[REDACTED: signedTransferTxBytes(<n>-bytes)]"}'
+ */
+export function errMessage(err: unknown): string {
+  if (err instanceof Error) return err.message;
+  if (typeof err === 'string') return err;
+  try {
+    return JSON.stringify(redactCause(err));
+  } catch {
+    return String(err);
+  }
+}

modules/payments/NametagMinter.ts

@@ -14,6 +14,7 @@
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import { logger } from '../../core/logger';
+import { errMessage } from '../../core/errors';
 
 import { Token } from '@unicitylabs/state-transition-sdk/lib/token/Token';
 import { TokenId } from '@unicitylabs/state-transition-sdk/lib/token/TokenId';
@@ -179,7 +180,7 @@ export class NametagMinter {
           if (attempt === MAX_RETRIES) {
             return {
               success: false,
-              error: `Submit failed: ${error instanceof Error ? error.message : String(error)}`,
+              error: `Submit failed: ${errMessage(error)}`,
             };
           }
           await new Promise(r => setTimeout(r, 1000 * attempt));
@@ -253,7 +254,7 @@ export class NametagMinter {
       this.log('Minting failed:', error);
       return {
         success: false,
-        error: error instanceof Error ? error.message : String(error),
+        error: errMessage(error),
       };
     }
   }

modules/payments/transfer/nostr-persistence-verifier.ts

@@ -86,7 +86,7 @@ import type { UxfSentLedgerEntry } from '../../../types/uxf-sent';
 import type { SentLedgerWriter } from '../../../profile/sent-ledger-writer';
 import type { OutboxWriter } from '../../../profile/outbox-writer';
 import type { UxfTransferOutboxEntry } from '../../../types/uxf-outbox';
-import { redactCause, SphereError } from '../../../core/errors';
+import { errMessage, SphereError } from '../../../core/errors';
 
 // =============================================================================
 // 1. Public types — dependency surface + options
@@ -637,12 +637,3 @@ function emptyResult(
   };
 }
 
-function errMessage(err: unknown): string {
-  if (err instanceof Error) return err.message;
-  if (typeof err === 'string') return err;
-  try {
-    return JSON.stringify(redactCause(err));
-  } catch {
-    return String(err);
-  }
-}

tests/unit/core/errors.errMessage.test.ts

@@ -0,0 +1,102 @@
+/**
+ * Issue #191 — lossy error stringification.
+ *
+ * The inline pattern `err instanceof Error ? err.message : String(err)`
+ * collapses object-shaped errors to `'[object Object]'`, masking aggregator
+ * response payloads. `errMessage` (in `core/errors.ts`) is the documented
+ * replacement — it preserves field-level forensics by JSON-stringifying
+ * unknown shapes through the W40 redaction layer.
+ *
+ * This test pins the contract callers depend on:
+ *  1. Error instances surface `.message` verbatim.
+ *  2. String throws surface unchanged.
+ *  3. Plain-object throws become JSON, NOT `'[object Object]'`.
+ *  4. Sensitive fields are still redacted (no W40 bypass).
+ *  5. Unstringifiable values fall back to `String(err)` without throwing.
+ */
+import { describe, expect, it } from 'vitest';
+
+import { errMessage } from '../../../core/errors';
+
+describe('errMessage()', () => {
+  it('returns Error.message for Error instances', () => {
+    expect(errMessage(new Error('boom'))).toBe('boom');
+  });
+
+  it('preserves custom Error subclass messages', () => {
+    class CustomError extends Error {}
+    expect(errMessage(new CustomError('custom-boom'))).toBe('custom-boom');
+  });
+
+  it('returns the value unchanged for string throws', () => {
+    expect(errMessage('plain-string')).toBe('plain-string');
+  });
+
+  it('JSON-stringifies plain objects instead of "[object Object]"', () => {
+    // The bug fix's headline test — issue #191's reproducer.
+    const out = errMessage({ status: 'BAD_REQUEST', reason: 'rate-limit' });
+    expect(out).not.toBe('[object Object]');
+    expect(out).toBe('{"status":"BAD_REQUEST","reason":"rate-limit"}');
+  });
+
+  it('serializes the aggregator-style structured payload', () => {
+    // The shape NametagMinter sees back from a rejected submit_commitment —
+    // matches the issue #191 expected output illustration.
+    const payload = {
+      status: 'BAD_REQUEST',
+      details: { code: 'rate-limit-exceeded', retryAfterMs: 5000 },
+    };
+    const out = errMessage(payload);
+    expect(out).toContain('"status":"BAD_REQUEST"');
+    expect(out).toContain('"retryAfterMs":5000');
+  });
+
+  it('routes through redactCause so sensitive fields never leak via errMessage', () => {
+    const out = errMessage({
+      tokenId: 't1',
+      signedTransferTxBytes: new Uint8Array([1, 2, 3, 4]),
+    });
+    expect(out).toContain('"tokenId":"t1"');
+    expect(out).toContain('[REDACTED: signedTransferTxBytes(4-bytes)]');
+    // Raw bytes never appear in the surfaced string.
+    expect(out).not.toMatch(/\[1,2,3,4\]/);
+  });
+
+  it('handles array throws by serializing each element', () => {
+    expect(errMessage([1, 'two', { k: 'v' }])).toBe('[1,"two",{"k":"v"}]');
+  });
+
+  it('handles null and undefined without throwing', () => {
+    // JSON.stringify(null) === 'null'; redactCause(undefined) === undefined
+    // and JSON.stringify(undefined) === undefined; the catch-fallback
+    // surfaces 'undefined' via String(err). The exact string is less
+    // important than the no-throw guarantee.
+    expect(errMessage(null)).toBe('null');
+    expect(() => errMessage(undefined)).not.toThrow();
+  });
+
+  it('falls back to String(err) for unstringifiable shapes (BigInt)', () => {
+    // BigInt throws on JSON.stringify — we MUST NOT propagate that throw.
+    const out = errMessage({ amount: 10n });
+    // Either path is acceptable: a redactCause walk that emits 'null' for
+    // BigInt, or the catch-fallback's `String(err)`. The contract is no
+    // throw and a non-empty string.
+    expect(typeof out).toBe('string');
+    expect(out.length).toBeGreaterThan(0);
+  });
+
+  it('handles self-referential causes without throwing (cycle-safe)', () => {
+    const cyclic: Record<string, unknown> = { name: 'parent' };
+    cyclic.self = cyclic;
+    // `redactCause` is cycle-safe (WeakMap visited set), but the redacted
+    // clone preserves the cycle structurally so `JSON.stringify` still
+    // throws. The catch-fallback to `String(err)` returns
+    // '[object Object]' — strictly worse than non-cyclic shapes, but
+    // acceptable: the headline bug (non-cyclic objects masked as
+    // '[object Object]') is already prevented by the earlier tests.
+    // The contract here is no-throw + non-empty string.
+    const out = errMessage(cyclic);
+    expect(typeof out).toBe('string');
+    expect(out.length).toBeGreaterThan(0);
+  });
+});