Fix regression issue of Codeberg PR 1112 #1

melroy89 · 2023-10-12T22:47:12Z

First PR 👍🏽

- ActivityHandler: Removed situation where many failed messages were just logged and cleared (not sent to failed) - ActivityHandler: When deleting a remote user, do not try to fetch the ap profile URL, it likely won't exist any more. - ChainActivityHandler: When null is returned from client getting Activity Object, raise an exception to send it to the failed queue - LinkEmbedHandler: Don't just try to add. Update existing link if already exists. Note, this can cause problems potentially. But, better than errors and not handling message properly - HttpStatusHelper: New class with functions to test status codes for temporary failures, or missing/deleted state. Used by ApHttpClient - ApHttpClient: Changed cases of InvalidApPostException for InvalidApGetException where appropriate. - ApHttpClient: If an error is a recoverable status code throw RecoverableMessageHandlingException. This should ensure the error'd message doesn't expire. - ApHttpClient: In many cases where an exception was thrown and included the payload. Now the exception shows the https response code but doesn't include the payload - the same error with payload is logged. This is to clean up the failed list - ApHttpClient: Don't treat all 4xx codes as missing. This isn't the case. 404/410 count as missing. Some 4xx/5xx codes are temporary failures and the rest are treated as a normal failure - ApHttpClient: Magazine save on generic exception (assumed as timeout) was trying to save the user (which is null) instead of magazine. - ActivityPubManager: When searching for local user/magazine, lookup by handle (apId) instead of apProfileId if available. ApProfileId sometimes changes from http to https - This causes the subsequent attempt to create a new user/magazine to fail because it's not really a new user and the apId matches - ChainActivityHandler: Only try to get entity from object if it is an array - Need to find out how bool is getting there though. Maybe happening when only null is in there and we try to get end? - LikeHandler: Don't try to undo a like if there isn't one to remove. - Resolve array_merge(): Argument MbinOrg#1 must be of type array, string given caused by lotide sending string To/CC when we expect an array - Created helper library for such functions and added function to merge a string or array into an array - ActivityHandler, ChainActivityHandler, LinkEmbedHandler, ApHttpClient, ActivityPubManager, HttpStatusHelper - Codestyle change for changes already made - LikeHandler - Some logging retained to trace source of "null is not callable" errors suspected to happen here - Note, Page, Question - Refactored StringHelper to ArrayHelper - ActivityPubManager - Changed to attempt to get handle via finger but fall back to ap_profile on any failure. Attempt to correct errors when http changes to https or vice-versa - Use isset to ensure finger result and actorHandle were actually set. Prevents a php warning - FavouriteFactory - Use symphony lock (semaphore/flock depending on symfony config) to ensure one like toggle is processed at once - Resolves issue where multiple likes on any of the four types could be duplicated if they happened in quick succession - ImageManager - Also check resource_type to detect closed file, checking on $fh alone isn't enough - ArrayHelper - New class supporting safe_merge_array which will accept an array, string or null on either or both sides and still merge into an array. - Resolves issue federating with lotide

before this fix you would get errors like this on `CreateMessage`s > (Symfony\\Component\\Messenger\\Exception\\HandlerFailedException(code: 0): Handling \"App\\Message\\ActivityPub\\Inbox\\CreateMessage\" failed: App\\Service\\ActivityPub\\MarkdownConverter::convert(): Argument #1 ($value) must be of type string, null given, called in /var/www/mbin/src/Service/ActivityPub/ApObjectExtractor.php on line 39 at /var/www/mbin/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php:124)(TypeError(code: 0): App\\Service\\ActivityPub\\MarkdownConverter::convert(): Argument #1 ($value) must be of type string, null given, called in /var/www/mbin/src/Service/ActivityPub/ApObjectExtractor.php on line 39 at /var/www/mbin/src/Service/ActivityPub/MarkdownConverter.php:22) e.g. when posting a link without a description (without a body). Since this code is able to handle null returns, we can just do that on a null body

occasionally this cryptic error turned up in failed messages: Handling "App\Message\ActivityPub\Inbox\LikeMessage" failed: strpos(): Argument MbinOrg#1 ($haystack) must be of type string, null given turns out this comes from null guard in Undo/Like handler where it checks actor and entity to not be null, but entity is resolved from activity which *could* be null when the target object isn't known to us, and trying to resolve the entity with that would results in the cryptic error above moved the empty check in Undo/Like handler from `$entity` to `$activity` since entity is resolved from activity and when the activity has any values then the target object entity exists and can be resolved

occasionally this cryptic error turned up in failed messages: Handling "App\Message\ActivityPub\Inbox\LikeMessage" failed: strpos(): Argument MbinOrg#1 ($haystack) must be of type string, null given turns out this comes from null guard in Undo/Like handler where it checks actor and entity to not be null, but entity is resolved from activity which *could* be null when the target object isn't known to us, and trying to resolve the entity with that would results in the cryptic error above moved the empty check in Undo/Like handler from `$entity` to `$activity` since entity is resolved from activity and when the activity has any values then the target object entity exists and can be resolved also applied this fix to the DislikeHandler as well

occasionally this cryptic error turned up in failed messages: Handling "App\Message\ActivityPub\Inbox\LikeMessage" failed: strpos(): Argument #1 ($haystack) must be of type string, null given turns out this comes from null guard in Undo/Like handler where it checks actor and entity to not be null, but entity is resolved from activity which *could* be null when the target object isn't known to us, and trying to resolve the entity with that would results in the cryptic error above moved the empty check in Undo/Like handler from `$entity` to `$activity` since entity is resolved from activity and when the activity has any values then the target object entity exists and can be resolved also applied this fix to the DislikeHandler as well

Fix regression issue of Codeberg PR 1112

32b958c

melroy89 merged commit dde777c into main Oct 12, 2023

TheVillageGuy mentioned this pull request Oct 18, 2023

New magazines after a search are created with deleted user and related problems #67

Closed

melroy89 deleted the fix_pr_1112-i branch October 26, 2023 01:01

BentiGorlich mentioned this pull request Feb 17, 2024

Fix error on entries with an empty body #504

Merged

melroy89 mentioned this pull request Sep 12, 2024

Miscellaneous error/warning log items #1119

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix regression issue of Codeberg PR 1112 #1

Fix regression issue of Codeberg PR 1112 #1

melroy89 commented Oct 12, 2023

Fix regression issue of Codeberg PR 1112 #1

Fix regression issue of Codeberg PR 1112 #1

Conversation

melroy89 commented Oct 12, 2023