Skip to content

Preserve Context Information for Exceptions Thrown Within Scopes #56365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SyuTingSong wants to merge 1 commit into from
Closed

Preserve Context Information for Exceptions Thrown Within Scopes #56365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/Illuminate/Log/Context/Repository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Illuminate\Support\Traits\Macroable;
use RuntimeException;
use Throwable;
use WeakMap;

class Repository
{
Expand Down Expand Up @@ -47,12 +48,18 @@ class Repository
*/
protected static $handleUnserializeExceptionsUsing;

/**
* @var WeakMap<Throwable, static>
*/
protected static $forThrowable;

/**
* Create a new Context instance.
*/
public function __construct(Dispatcher $events)
{
$this->events = $events;
self::$forThrowable ??= new WeakMap();
}

/**
Expand Down Expand Up @@ -557,12 +564,27 @@ public function scope(callable $callback, array $data = [], array $hidden = [])

try {
return $callback();
} catch (Throwable $e) {
self::$forThrowable[$e] ??= (new static($this->events))
->add($this->all())
->addHidden($this->allHidden());

throw $e;
} finally {
$this->data = $dataBefore;
$this->hidden = $hiddenBefore;
}
}

/**
* @param Throwable $e
* @return static
*/
public function for(Throwable $e)
{
return self::$forThrowable[$e] ?? $this;
}
Comment on lines +583 to +586
Copy link
Contributor

@cosmastech cosmastech Jul 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: Quite a bit of synchronicity as we are working on recording contextual information in DataDog error tracking.

I think that the developer ergonomics here are less than ideal. Any time I want to report my exception (or let the container report it), I have to remember to reset the context from the exception. It feels more practical that the container would manage restoring the Context state for the exception.

Copy link
Contributor

@cosmastech cosmastech Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this can be done inside of Illuminate\Support\Foundation\Exceptions\Handler@exceptionContext()?

Copy link
Author

@SyuTingSong SyuTingSong Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely agree that the exception Context retrieval should be built into framework public methods like Handler, Log, dd(), etc., without requiring developers to do anything manually. I didn't include those changes in the current PR mainly because I wanted to avoid making the changeset too large, and wanted to gauge the maintainers' attitude before deciding whether to complete everything in a single PR.


/**
* Determine if the repository is empty.
*
Expand Down
66 changes: 66 additions & 0 deletions tests/Log/ContextTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,72 @@ public function test_it_remembers_a_hidden_value()
Context::rememberHidden('foo', $closure);
$this->assertSame(1, $closureRunCount);
}

public function test_it_can_restore_scope_context_for_throwable()
{
Context::add('foo', 'bar');
Context::addHidden('hello', 'world');

try {
Context::scope(function () {
Context::add('foo', 'bar2');
Context::addHidden('hello', 'world3');

throw new RuntimeException('Test exception');
});
} catch (RuntimeException $e) {
$context = Context::for($e);
$this->assertSame('bar2', $context->get('foo'));
$this->assertSame('world3', $context->getHidden('hello'));
}
}

public function test_it_can_fallback_to_global_context_for_none_scoped_throwable()
{
Context::add('foo', 'bar');
Context::addHidden('hello', 'world');

try {
Context::scope(function () {
Context::add('foo', 'bar2');
Context::addHidden('hello', 'world3');
});

throw new RuntimeException('Test exception');
} catch (RuntimeException $e) {
$context = Context::for($e);
$this->assertSame('bar', $context->get('foo'));
$this->assertSame('world', $context->getHidden('hello'));
}
}

public function test_it_can_restore_innermost_scope_context_for_nested_throwable()
{
Context::add('foo', 'bar');
Context::addHidden('hello', 'world');

try {
Context::scope(function () {
Context::add('foo', 'bar2');
Context::addHidden('hello', 'world2');

Context::scope(function () {
Context::add('foo', 'bar3');
Context::addHidden('hello', 'world3');

throw new RuntimeException('Nested exception');
});
});
} catch (RuntimeException $e) {
$context = Context::for($e);
$this->assertSame('bar3', $context->get('foo'));
$this->assertSame('world3', $context->getHidden('hello'));
}

// Verify global context is restored
$this->assertSame('bar', Context::get('foo'));
$this->assertSame('world', Context::getHidden('hello'));
}
}

enum Suit
Expand Down
Loading