Skip to content
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

Jump to bottom

Reduce error pollution even more on post request failures #1140

Merged
merged 12 commits into from
Sep 23, 2024
Merged

Reduce error pollution even more on post request failures #1140

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4c0174b
Reduce error pollution even more on post request failures
melroy89 Sep 19, 2024
df86ed4
Putting the build cache outside of the try/cache
melroy89 Sep 19, 2024
9cfe44b
Remove empty line
melroy89 Sep 19, 2024
d526643
Reduce scope of var, just like the rest of the code
melroy89 Sep 19, 2024
73d9c82
Small refactoring
melroy89 Sep 19, 2024
bc3eea8
Merge branch 'main' into reduce_error_pollution_more
melroy89 Sep 20, 2024
b37124d
Add code docs + remove the word "get". Since it can both be a get or …
melroy89 Sep 20, 2024
2202c6c
And debug log
melroy89 Sep 20, 2024
3c0d4b2
Update ApHttpClient.php
melroy89 Sep 20, 2024
48bf900
Fix throw docs
melroy89 Sep 20, 2024
306a3d7
Merge branch 'main' into reduce_error_pollution_more
melroy89 Sep 21, 2024
0f8cfba
Merge branch 'main' into reduce_error_pollution_more
melroy89 Sep 23, 2024
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
70 changes: 44 additions & 26 deletions src/Service/ActivityPub/ApHttpClient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,27 +89,27 @@ public function getActivityObject(string $url, bool $decoded = true): array|stri
private function getActivityObjectImpl(string $url): ?string
{
$this->logger->debug("ApHttpClient:getActivityObject:url: $url");

$client = new CurlHttpClient();
$content = null;
try {
$r = $client->request('GET', $url, [
$client = new CurlHttpClient();
$response = $client->request('GET', $url, [
'max_duration' => self::MAX_DURATION,
'timeout' => self::TIMEOUT,
'headers' => $this->getInstanceHeaders($url),
]);

$statusCode = $r->getStatusCode();
$statusCode = $response->getStatusCode();
// Accepted status code are 2xx or 410 (used Tombstone types)
if (!str_starts_with((string) $statusCode, '2') && 410 !== $statusCode) {
// Do NOT include the content in the error message, this will be often a full HTML page
throw new InvalidApPostException("Invalid status code while getting: $url : $statusCode");
// Do NOT include the response content in the error message, this will be often a full HTML page
throw new InvalidApPostException("Invalid status code while getting: $url, status code: $statusCode");
}

// Read also non-OK responses (like 410) by passing 'false'
$content = $r->getContent(false);
$content = $response->getContent(false);
$this->logger->debug('ApHttpClient:getActivityObject:url: {url} - content: {content}', ['url' => $url, 'content' => $content]);
} catch (\Exception $e) {
$this->logRequestException($r, $url, 'ApHttpClient:getActivityObject', $e);
$this->logRequestException($response, $url, 'ApHttpClient:getActivityObject', $e);
}

return $content;
Expand Down Expand Up @@ -171,19 +171,19 @@ public function getWebfingerObject(string $url): ?array
private function getWebfingerObjectImpl(string $url): ?string
{
$this->logger->debug("ApHttpClient:getWebfingerObject:url: $url");
$r = null;
$response = null;
try {
$client = new CurlHttpClient();
$r = $client->request('GET', $url, [
$response = $client->request('GET', $url, [
'max_duration' => self::MAX_DURATION,
'timeout' => self::TIMEOUT,
'headers' => $this->getInstanceHeaders($url, null, 'get', ApRequestType::WebFinger),
]);
} catch (\Exception $e) {
$this->logRequestException($r, $url, 'ApHttpClient:getWebfingerObject', $e);
$this->logRequestException($response, $url, 'ApHttpClient:getWebfingerObject', $e);
}

return $r->getContent();
return $response->getContent();
}

private function getActorCacheKey(string $apProfileId): string
Expand Down Expand Up @@ -317,8 +317,8 @@ private function getCollectionObjectImpl(string $apAddress): ?string
$statusCode = $response->getStatusCode();
// Accepted status code are 2xx or 410 (used Tombstone types)
if (!str_starts_with((string) $statusCode, '2') && 410 !== $statusCode) {
// Do NOT include the content in the error message, this will be often a full HTML page
throw new InvalidApPostException("Invalid status code while getting: $apAddress : $statusCode");
// Do NOT include the response content in the error message, this will be often a full HTML page
throw new InvalidApPostException("Invalid status code while getting: $apAddress, status code: $statusCode");
}
} catch (\Exception $e) {
$this->logRequestException($response, $apAddress, 'ApHttpClient:getCollectionObject', $e);
Expand All @@ -328,6 +328,16 @@ private function getCollectionObjectImpl(string $apAddress): ?string
return $response->getContent();
}

/**
* Helper function for logging get/post/.. requests to the error & debug log with additional info.
*
* @param ResponseInterface|null $response Optional response object
* @param string $requestUrl Full URL of the request
* @param string $requestType an additional string where the error happened in the code
* @param \Exception $e Error object
*
* @throws InvalidApPostException rethrows the error
*/
private function logRequestException(?ResponseInterface $response, string $requestUrl, string $requestType, \Exception $e): void
{
if (null !== $response) {
Expand All @@ -341,7 +351,7 @@ private function logRequestException(?ResponseInterface $response, string $reque

// Often 400, 404 errors just return the full HTML page, so we don't want to log the full content of them
// We truncate the content to 200 characters max.
$this->logger->error('{type} get fail: {address}, ex: {e}: {msg}. Truncated content: {content}', [
$this->logger->error('{type} failed: {address}, ex: {e}: {msg}. Truncated content: {content}', [
'type' => $requestType,
'address' => $requestUrl,
'e' => \get_class($e),
Expand All @@ -354,7 +364,7 @@ private function logRequestException(?ResponseInterface $response, string $reque
'content' => $content,
]);
}
throw $e;
throw $e; // re-throw the exception
}

/**
Expand All @@ -379,20 +389,28 @@ public function post(string $url, User|Magazine $actor, ?array $body = null): vo
return;
}

$jsonBody = json_encode($body ?? []);

$this->logger->debug("ApHttpClient:post:url: $url");
$this->logger->debug('ApHttpClient:post:body '.json_encode($body ?? []));
$this->logger->debug("ApHttpClient:post:body $jsonBody");

// Set-up request
$client = new CurlHttpClient();
$response = $client->request('POST', $url, [
'max_duration' => self::MAX_DURATION,
'timeout' => self::TIMEOUT,
'body' => json_encode($body),
'headers' => $this->getHeaders($url, $actor, $body),
]);
try {
$client = new CurlHttpClient();
$response = $client->request('POST', $url, [
'max_duration' => self::MAX_DURATION,
'timeout' => self::TIMEOUT,
'body' => $jsonBody,
'headers' => $this->getHeaders($url, $actor, $body),
]);

if (!str_starts_with((string) $response->getStatusCode(), '2')) {
throw new InvalidApPostException("Post fail: $url, ".substr($response->getContent(false), 0, 1000).' '.json_encode($body));
$statusCode = $response->getStatusCode();
if (!str_starts_with((string) $statusCode, '2')) {
// Do NOT include the response content in the error message, this will be often a full HTML page
throw new InvalidApPostException("Post failed: $url, status code: $statusCode, request body: $jsonBody");
}
} catch (\Exception $e) {
$this->logRequestException($response, $url, 'ApHttpClient:post', $e);
melroy89 marked this conversation as resolved.
Show resolved Hide resolved
}

// build cache
Expand Down
Loading