Skip to content

Commit 344dd1f

Browse files
allouisallouis
committed
Migrated getByApId to return a Result
ref https://linear.app/ghost/issue/AP-1045 This uses our new Result type to make sure that we handle all possible errors, and we no longer return 500's for any of these "expected" errors. If/when we find that there are still 500's created from this flow, we should explicitly catch and return these errors and handle them
1 parent 110a3d5 commit 344dd1f

File tree

7 files changed

+424
-73
lines changed

7 files changed

+424
-73
lines changed

‎src/activity-handlers/create.handler.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Context, Create } from '@fedify/fedify';
22
import type { AccountService } from 'account/account.service';
33
import type { ContextData } from 'app';
4+
import { exhaustiveCheck, getError, isError } from 'core/result';
45
import { isFollowedByDefaultSiteAccount } from 'helpers/activitypub/actor';
56
import { getUserData } from 'helpers/user';
67
import { addToList } from 'kv-helpers';
@@ -35,7 +36,39 @@ export class CreateHandler {
3536
}
3637

3738
// This handles storing the posts in the posts table
38-
const post = await this.postService.getByApId(create.objectId);
39+
const postResult = await this.postService.getByApId(create.objectId);
40+
41+
if (isError(postResult)) {
42+
const error = getError(postResult);
43+
switch (error) {
44+
case 'upstream-error':
45+
ctx.data.logger.info(
46+
'Upstream error fetching post for create handling',
47+
{
48+
postId: create.objectId.href,
49+
},
50+
);
51+
break;
52+
case 'not-a-post':
53+
ctx.data.logger.info(
54+
'Resource is not a post in create handling',
55+
{
56+
postId: create.objectId.href,
57+
},
58+
);
59+
break;
60+
case 'missing-author':
61+
ctx.data.logger.info(
62+
'Post has missing author in create handling',
63+
{
64+
postId: create.objectId.href,
65+
},
66+
);
67+
break;
68+
default:
69+
return exhaustiveCheck(error);
70+
}
71+
}
3972

4073
const createJson = await create.toJsonLd();
4174
ctx.data.globaldb.set([create.id.href], createJson);

‎src/activity-handlers/delete.handler.ts‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { Actor, Context, Delete } from '@fedify/fedify';
22
import type { Account } from 'account/account.entity';
33
import type { AccountService } from 'account/account.service';
44
import type { ContextData } from 'app';
5-
import type { Post } from 'post/post.entity';
5+
import { exhaustiveCheck, getError, getValue, isError } from 'core/result';
66
import type { KnexPostRepository } from 'post/post.repository.knex';
77
import type { PostService } from 'post/post.service';
88
import { getRelatedActivities } from '../db';
@@ -59,20 +59,23 @@ export class DeleteHandler {
5959
return;
6060
}
6161

62-
let post: Post | null = null;
63-
64-
try {
65-
post = await this.postService.getByApId(deleteActivity.objectId);
66-
} catch (error) {
67-
ctx.data.logger.error('Error fetching post', { error });
68-
return;
69-
}
62+
const postResult = await this.postService.getByApId(
63+
deleteActivity.objectId,
64+
);
7065

71-
if (post === null) {
72-
ctx.data.logger.info('Post not found, exit early');
73-
return;
66+
if (isError(postResult)) {
67+
const error = getError(postResult);
68+
switch (error) {
69+
case 'upstream-error':
70+
case 'missing-author':
71+
case 'not-a-post':
72+
return;
73+
default:
74+
return exhaustiveCheck(error);
75+
}
7476
}
7577

78+
const post = getValue(postResult);
7679
post.delete(senderAccount);
7780
await this.postRepository.save(post);
7881

‎src/dispatchers.ts‎

Lines changed: 138 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
import * as Sentry from '@sentry/node';
2222
import type { KnexAccountRepository } from 'account/account.repository.knex';
2323
import type { FollowersService } from 'activitypub/followers.service';
24+
import { exhaustiveCheck, getError, getValue, isError } from 'core/result';
2425
import { v4 as uuidv4 } from 'uuid';
2526
import type { AccountService } from './account/account.service';
2627
import { mapActorToExternalAccountData } from './account/utils';
@@ -371,7 +372,39 @@ export async function handleAnnoucedCreate(
371372
return;
372373
}
373374
// This handles storing the posts in the posts table
374-
const post = await postService.getByApId(create.objectId);
375+
const postResult = await postService.getByApId(create.objectId);
376+
377+
if (isError(postResult)) {
378+
const error = getError(postResult);
379+
switch (error) {
380+
case 'upstream-error':
381+
ctx.data.logger.info(
382+
'Upstream error fetching post for create handling',
383+
{
384+
postId: create.objectId.href,
385+
},
386+
);
387+
break;
388+
case 'not-a-post':
389+
ctx.data.logger.info(
390+
'Resource is not a post in create handling',
391+
{
392+
postId: create.objectId.href,
393+
},
394+
);
395+
break;
396+
case 'missing-author':
397+
ctx.data.logger.info(
398+
'Post has missing author in create handling',
399+
{
400+
postId: create.objectId.href,
401+
},
402+
);
403+
break;
404+
default:
405+
exhaustiveCheck(error);
406+
}
407+
}
375408

376409
const object = await create.getObject();
377410
const replyTarget = await object?.getReplyTarget();
@@ -452,14 +485,45 @@ export const createUndoHandler = (
452485
}
453486

454487
if (senderAccount !== null) {
455-
const originalPost = await postService.getByApId(
488+
const originalPostResult = await postService.getByApId(
456489
object.objectId,
457490
);
458491

459-
if (originalPost !== null) {
460-
originalPost.removeRepost(senderAccount);
461-
await postRepository.save(originalPost);
492+
if (isError(originalPostResult)) {
493+
const error = getError(originalPostResult);
494+
switch (error) {
495+
case 'upstream-error':
496+
ctx.data.logger.info(
497+
'Upstream error fetching post for undoing announce',
498+
{
499+
postId: object.objectId.href,
500+
},
501+
);
502+
break;
503+
case 'not-a-post':
504+
ctx.data.logger.info(
505+
'Resource is not a post in undoing announce',
506+
{
507+
postId: object.objectId.href,
508+
},
509+
);
510+
break;
511+
case 'missing-author':
512+
ctx.data.logger.info(
513+
'Post has missing author in undoing announce',
514+
{
515+
postId: object.objectId.href,
516+
},
517+
);
518+
break;
519+
default:
520+
return exhaustiveCheck(error);
521+
}
522+
return;
462523
}
524+
const originalPost = getValue(originalPostResult);
525+
originalPost.removeRepost(senderAccount);
526+
await postRepository.save(originalPost);
463527
}
464528
}
465529

@@ -590,9 +654,40 @@ export function createAnnounceHandler(
590654

591655
if (senderAccount !== null) {
592656
// This will save the post if it doesn't already exist
593-
const post = await postService.getByApId(announce.objectId);
594-
595-
if (post !== null) {
657+
const postResult = await postService.getByApId(announce.objectId);
658+
659+
if (isError(postResult)) {
660+
const error = getError(postResult);
661+
switch (error) {
662+
case 'upstream-error':
663+
ctx.data.logger.info(
664+
'Upstream error fetching post for reposting',
665+
{
666+
postId: announce.objectId.href,
667+
},
668+
);
669+
break;
670+
case 'not-a-post':
671+
ctx.data.logger.info(
672+
'Resource for reposting is not a post',
673+
{
674+
postId: announce.objectId.href,
675+
},
676+
);
677+
break;
678+
case 'missing-author':
679+
ctx.data.logger.info(
680+
'Post for reposting has missing author',
681+
{
682+
postId: announce.objectId.href,
683+
},
684+
);
685+
break;
686+
default:
687+
return exhaustiveCheck(error);
688+
}
689+
} else {
690+
const post = getValue(postResult);
596691
post.addRepost(senderAccount);
597692
await postRepository.save(post);
598693
}
@@ -637,11 +732,42 @@ export function createLikeHandler(
637732

638733
const account = await accountService.getByApId(like.actorId);
639734
if (account !== null) {
640-
const post = await postService.getByApId(like.objectId);
641-
642-
if (post !== null) {
735+
const postResult = await postService.getByApId(like.objectId);
736+
737+
if (isError(postResult)) {
738+
const error = getError(postResult);
739+
switch (error) {
740+
case 'upstream-error':
741+
ctx.data.logger.info(
742+
'Upstream error fetching post for liking',
743+
{
744+
postId: like.objectId.href,
745+
},
746+
);
747+
break;
748+
case 'not-a-post':
749+
ctx.data.logger.info(
750+
'Resource for liking is not a post',
751+
{
752+
postId: like.objectId.href,
753+
},
754+
);
755+
break;
756+
case 'missing-author':
757+
ctx.data.logger.info(
758+
'Post for liking has missing author',
759+
{
760+
postId: like.objectId.href,
761+
},
762+
);
763+
break;
764+
default: {
765+
return exhaustiveCheck(error);
766+
}
767+
}
768+
} else {
769+
const post = getValue(postResult);
643770
post.addLike(account);
644-
645771
await postRepository.save(post);
646772
}
647773
}

0 commit comments

Comments
 (0)