Skip to content

fix: preserve chained exception cause in logException#7

Open
alexstandiford wants to merge 1 commit into
mainfrom
fix/log-exception-preserve-chained-cause
Open

fix: preserve chained exception cause in logException#7
alexstandiford wants to merge 1 commit into
mainfrom
fix/log-exception-preserve-chained-cause

Conversation

@alexstandiford

@alexstandiford alexstandiford commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Problem

CanLogException::logException() logs only the outer exception's message and assigns the exception object to context['exception'] — it never walks getPrevious().

This became a silent data-loss bug once infrastructure exceptions started carrying their real cause as a chained previous exception. The PDO datastore backend (phpnomad/mysql-integration) deliberately throws:

throw new DatastoreErrorException('Failed to execute query.', 500, $e); // real PDOException chained as $previous

The generic outer message is intentional (SQL/driver detail must not reach clients), with the actual cause carried on the chain for loggers. But logException discarded the chain, so every datastore failure logged:

[CRITICAL] - Failed to execute query.

…with the real error (column not found, duplicate key, constraint violation) gone. Downstream, consumers like Siren have ~360 logException($e) callsites that all went blind at once. The exception object in context['exception'] doesn't help listeners that json_encode context — an Exception serializes to {}.

Fix

Walk the $e->getPrevious() chain and fold each non-empty message into the logged line. Consumer usage doesn't change — bare logException($e) now surfaces the real cause automatically.

Folding the whole chain naively has two failure modes, both handled:

  • Duplication — some wrappers embed the cause's message in their own text and chain the cause (e.g. QueryStrategy: 'Invalid query: ' . $e->getMessage() with $e as previous). A level is skipped when its message is already contained in a collected part, so it isn't logged twice.
  • Unbounded growth — the walk is bounded by a depth cap ($maxLoggedExceptionDepth, default 10) plus a cycle guard, so a pathological chain can't produce a giant message.

Also drops the spurious leading " - " separator that appeared when no prefix message was supplied.

Before:

 - Failed to execute query.

After:

Failed to execute query. - SQLSTATE[42S22]: Column not found: 1054 Unknown column "foo"

Tests

Adds CanLogExceptionTest (11 tests): default/explicit level, context preservation, single- and multi-level chain flattening, the duplication-skip case, the depth cap, and the leading-separator fix. Full suite green; cs-fixer clean.

Version bumped `1.2.1 → 1.2.2`.

logException previously logged only the outer exception's message and
discarded the chain. Infrastructure exceptions (e.g. the PDO datastore
backend) deliberately surface a stable, client-safe message and carry
the real cause as a chained previous exception, so every datastore
failure logged only "Failed to execute query." with the actual SQL
error dropped.

Walk the $e->getPrevious() chain and fold each non-empty message into
the logged line. A level is skipped when its message is already embedded
in a collected part (some wrappers embed the cause's text and also chain
it), and the walk is bounded by a depth cap plus a cycle guard so the
message can't grow without limit. Also drop the leading " - " separator
that appeared when no prefix message was supplied.
@alexstandiford alexstandiford force-pushed the fix/log-exception-preserve-chained-cause branch from 5ec244f to c49e928 Compare June 16, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant