Address Copilot review: normalize endpoints, quieter logs, robust tests

- resolveHandleToDid: rtrim trailing slash on the service URL before building
  the request URL and before the `=== $service` auth check (avoids double-slash
  URLs and mis-detected auth).
- Log per-endpoint resolution attempts at debug; emit a single warning only
  after all endpoints are exhausted (less noise during transient outages).
- Tests: fake resolveHandle with a wildcard so the case is isolated from the
  configured service URL; read request payload via $request->data().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Patrick Schrall 2026-06-11 17:37:16 +02:00
parent eaef12b331
commit b317b8dbb2
2 changed files with 24 additions and 4 deletions

View file

@ -278,12 +278,18 @@ private function parseFacets(string $text, ?string $service = null, ?SocialAccou
*/
private function resolveHandleToDid(string $handle, ?string $service = null, ?SocialAccount $account = null): ?string
{
// Normalize so a trailing slash neither produces a double-slash URL nor
// breaks the `=== $service` check below (which decides authentication).
$service = $service !== null ? rtrim($service, '/') : null;
$endpoints = array_values(array_unique(array_filter([
$service,
'https://public.api.bsky.app',
'https://bsky.social',
])));
$lastError = null;
$lastEndpoint = null;
foreach ($endpoints as $endpoint) {
try {
$request = $this->socialHttp();
@ -301,7 +307,12 @@ private function resolveHandleToDid(string $handle, ?string $service = null, ?So
return $did;
}
} catch (\Throwable $e) {
Log::warning('Bluesky handle resolution failed', [
// Per-endpoint failures are expected during transient outages;
// keep them at debug to avoid noisy logs and surface a single
// warning below only when every endpoint has been exhausted.
$lastError = $e->getMessage();
$lastEndpoint = $endpoint;
Log::debug('Bluesky handle resolution attempt failed', [
'handle' => $handle,
'endpoint' => $endpoint,
'error' => $e->getMessage(),
@ -309,6 +320,12 @@ private function resolveHandleToDid(string $handle, ?string $service = null, ?So
}
}
Log::warning('Bluesky handle resolution failed; mention will be sent as plain text', [
'handle' => $handle,
'endpoint' => $lastEndpoint,
'error' => $lastError,
]);
return null;
}

View file

@ -105,7 +105,10 @@
$this->post->update(['content' => 'Shout out to @friend.bsky.social']);
Http::fake([
'https://bsky.social/xrpc/com.atproto.identity.resolveHandle*' => Http::response([
// Wildcard so the fake matches whichever endpoint resolveHandleToDid()
// tries first (the account PDS, public AppView, or bsky.social) and the
// test stays isolated from the configured service URL.
'*/xrpc/com.atproto.identity.resolveHandle*' => Http::response([
'did' => 'did:plc:friend456',
], 200),
'https://bsky.social/xrpc/com.atproto.repo.createRecord' => Http::response([
@ -117,7 +120,7 @@
$this->publisher->publish($this->postPlatform);
Http::assertSent(function ($request) {
$record = $request['record'] ?? null;
$record = $request->data()['record'] ?? null;
if (! $record || ! isset($record['facets'])) {
return false;
@ -152,7 +155,7 @@
expect($result['id'])->toBe('3abc123xyz');
Http::assertSent(function ($request) {
$record = $request['record'] ?? null;
$record = $request->data()['record'] ?? null;
if (! $record) {
return false;