Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 34 additions & 12 deletions src/ToggleManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use OffloadProject\Toggle\Contracts\Driver;
use OffloadProject\Toggle\Exceptions\ToggleNotFoundException;
use RuntimeException;
use Throwable;

class ToggleManager
{
Expand All @@ -33,11 +34,20 @@ public function active(string|BackedEnum $name): bool
$key = $this->normalizeKey($name);

if ($this->cacheEnabled()) {
return $this->getCache()->remember(
$this->cacheKey($key),
$this->cacheTtl(),
fn () => $this->resolve($key)
);
try {
return $this->getCache()->remember(
$this->cacheKey($key),
$this->cacheTtl(),
fn () => $this->resolve($key)
);
} catch (Throwable $exception) {
// Rethrow toggle-specific exceptions so that genuine bugs are not masked.
if ($exception instanceof ToggleNotFoundException) {
throw $exception;
}
// Other exceptions (QueryException, PDOException, etc.) indicate cache
// unavailability - fall through to resolve without cache
}
}

return $this->resolve($key);
Expand Down Expand Up @@ -113,22 +123,34 @@ public function forgetCache(string|BackedEnum $name): bool
{
$key = $this->normalizeKey($name);

return $this->getCache()->forget($this->cacheKey($key));
try {
return $this->getCache()->forget($this->cacheKey($key));
Comment on lines +126 to +127

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

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

The forgetCache() method wraps getCache() in a try-catch, but getCache() is also called in the enable(), disable(), and delete() methods at lines 69, 84, and 99 without error handling. If cache is unavailable when these methods are called, they will throw an exception. Consider adding similar error handling to these call sites or ensuring forgetCache() can be called safely in all contexts.

Copilot uses AI. Check for mistakes.
} catch (Throwable) {
// Cache unavailable
return false;
}
Comment thread
shavonn marked this conversation as resolved.
}

/**
* Clear cache for all toggles.
*/
public function flushCache(): bool
{
$cache = $this->getCache();
// Get toggles from driver first (let driver exceptions propagate)
$toggles = $this->getDriver()->all();

// Clear all known toggles from cache
foreach ($this->getDriver()->all() as $name => $value) {
$cache->forget($this->cacheKey($name));
}
try {
$cache = $this->getCache();

return true;
foreach ($toggles as $name => $value) {
$cache->forget($this->cacheKey($name));
}

return true;
} catch (Throwable) {
// Cache unavailable
return false;
}
Comment on lines +150 to +153

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

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

Catching all Throwable instances is too broad. This catch block could also suppress errors from getDriver()->all() which is unrelated to cache operations. Consider catching more specific exceptions related to cache failures, or restructuring to only wrap the cache-specific operations.

Copilot uses AI. Check for mistakes.
}

/**
Expand Down
36 changes: 36 additions & 0 deletions tests/Feature/CacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

declare(strict_types=1);

use Illuminate\Database\QueryException;
use Illuminate\Support\Facades\Cache;
use OffloadProject\Toggle\Facades\Toggle;

Expand Down Expand Up @@ -39,3 +40,38 @@
expect(Cache::has('toggle:test-flag'))->toBeFalse();
expect(Cache::has('toggle:disabled-flag'))->toBeFalse();
});

it('gracefully handles cache unavailability during active check', function () {
// Verify caching is enabled for this test
expect(config('toggle.cache.enabled'))->toBeTrue();

// Mock cache to throw a QueryException (simulating database not existing)
$exception = new QueryException('sqlite', 'select * from cache', [], new Exception('Database does not exist'));

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

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

The Exception class is used without a proper import statement or full namespace qualifier. Add use Exception; to the imports at the top of the file, or use \Exception in the code.

Copilot uses AI. Check for mistakes.
Cache::shouldReceive('store')
->atLeast()->once()
->andThrow($exception);

// Should still resolve the toggle without cache
expect(Toggle::active('test-flag'))->toBeTrue();
expect(Toggle::active('disabled-flag'))->toBeFalse();
});

it('gracefully handles cache unavailability during forgetCache', function () {
$exception = new QueryException('sqlite', 'select * from cache', [], new Exception('Database does not exist'));

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

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

The Exception class is used without a proper import statement or full namespace qualifier. Add use Exception; to the imports at the top of the file, or use \Exception in the code.

Copilot uses AI. Check for mistakes.
Cache::shouldReceive('store')
->once()
->andThrow($exception);

// Should return false but not throw
expect(Toggle::forgetCache('test-flag'))->toBeFalse();
});

it('gracefully handles cache unavailability during flushCache', function () {
$exception = new QueryException('sqlite', 'select * from cache', [], new Exception('Database does not exist'));

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

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

The Exception class is used without a proper import statement or full namespace qualifier. Add use Exception; to the imports at the top of the file, or use \Exception in the code.

Copilot uses AI. Check for mistakes.
Cache::shouldReceive('store')
->once()
->andThrow($exception);

// Should return false but not throw
expect(Toggle::flushCache())->toBeFalse();
});