## Problem/Motivation `\Drupal\idle_reauthenticate\Controller\PingBack` writes to the private temp store in four places and deletes from it in a fifth, but never handles `\Drupal\Core\TempStore\TempStoreException`: | Line | Call | Key | |------|------|-----| | `PingBack.php:165` | `set()` | `lastSeen` | | `PingBack.php:178` | `set()` | `lastSeen` | | `PingBack.php:200` | `set()` | `state` (blocked) | | `PingBack.php:230` | `set()` | `state` (unblocked) | | `PingBack.php:247` | `delete()` | `state` | `PrivateTempStore::set()` and `PrivateTempStore::delete()` throw `TempStoreException` when the lock backend cannot be acquired. Core already calls `wait()` and retries once before throwing, so the exception means sustained lock contention on that key. It is a transient condition, never a programming error and never a data error. ### The controller currently fails open In `blockSession()` the `set()` on line 200 runs *before* the response commands that lock the screen (lines 202-222). If that write throws: 1. `SettingsCommand` is never added, so `currentState` is never set to `blocked` in `drupalSettings`. 2. `OpenModalDialogCommand` is never added, so the re-authentication dialog never opens. 3. The request returns a 500. 4. `js/idle_reauthenticate.js` deliberately swallows AJAX errors (lines 58-62, and the `fail()` handler on lines 65-67) and the `setTimeout` on line 68 keeps the polling loop alive. The net effect is that under lock contention an idle session is silently *not* locked, and nothing is written to the log for anyone to notice. The `set()` on line 165 has the same shape: it can throw before `blockSession()` is ever reached. For a module whose whole purpose is to lock an idle session, failing open is the wrong direction. Catching the exception and continuing makes it fail closed, because every one of the five call sites is safe to skip: - `lastSeen` (lines 165, 178): self-healing. The next ping rewrites it seconds later. - `state` -> blocked (line 200): this tab still shows the dialog; other tabs reach their own idle timeout independently. - `state` -> unblocked (line 230): other tabs stay blocked until they re-authenticate. Inconvenient, not insecure. - `delete()` (line 247): an orphaned entry that expires on its own. ### Secondary defect: the `delete()` in `reLogin()` is a no-op `reLogin()` calls `user_logout()` and only afterwards `$this->store->delete($this->getKey('state'))`. At that point the delete can never match anything, for two independent reasons: 1. `user_logout()` destroys the session, so `$this->session->getId()` inside `getKey()` no longer returns the value that was used when the entry was written, and the key does not exist. 2. `user_logout()` resets the account to anonymous, so `PrivateTempStore::getOwner()` no longer matches `$object->owner`, and `delete()` returns `FALSE` without touching storage. So the intended cleanup never happens and the entry is left to expire. ## Proposed resolution 1. Add two private helpers to `PingBack` that hold the error policy in one place, absorb the `getKey()` prefixing, and return whether the write succeeded: ```php use Drupal\Core\TempStore\TempStoreException; use Psr\Log\LogLevel; /** * Writes a value to the temp store, tolerating lock contention. * * @param string $key * The session scoped key, without the session prefix. * @param mixed $value * The value to store. * @param string $level * The severity to log a failed write with. * * @return bool * TRUE if the value was written, FALSE if the lock was unavailable. */ private function storeSet(string $key, mixed $value, string $level = LogLevel::WARNING): bool { try { $this->store->set($this->getKey($key), $value); return TRUE; } catch (TempStoreException $e) { $this->getLogger('idle_reauthenticate')->log($level, "Could not write '@key' to the temp store: @message", ['@key' => $key, '@message' => $e->getMessage()], ); return FALSE; } } ``` Plus an equivalent `storeDelete()`. `ControllerBase` already uses `LoggerChannelTrait`, so `$this->getLogger()` needs no constructor change. 2. Log the two key types at different severities. A failed `lastSeen` write is benign and happens on every ping, so logging it at `warning` would flood watchdog with one entry per user per ping interval; `info` is enough. A failed `state` write is worth a `warning`. 3. Move the `delete()` in `reLogin()` to *before* `user_logout()` so it can actually find and remove the entry. 4. Do not add an application-level retry loop. `PrivateTempStore` already waits and retries once, so an exception means contention that will not clear in microseconds. 5. Do not surface anything to the user. `messenger()->addError()` renders nothing in an AJAX heartbeat, and re-throwing produces the silent fail-open described above. ## Remaining tasks - [ ] Add `storeSet()` and `storeDelete()` helpers. - [ ] Convert the five call sites, using `LogLevel::INFO` for `lastSeen`. - [ ] Move the `delete()` in `reLogin()` ahead of `user_logout()`. - [ ] Add a kernel test that stubs the lock backend to refuse `acquire()` and asserts that the dialog still opens and no exception escapes. - [ ] Run phpcs and phpstan. ## User interface changes None. ## API changes None. ## Data model changes None. --- AI-Generated: Yes (Used OpenCode to analyze the controller, trace the exception paths through `PrivateTempStore`, `user_logout()` and the module JavaScript, and draft this issue summary.)