Diff Detail
- Repository
- rCOMM Comm
- Branch
- farcaster7
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
I am skeptical of this approach. The core issue is that our FID extraction breaks when unexpected object fields are encountered. What happens if Farcaster introducing a new object field? Won’t the same issue occur? I think we need to fix the core issue instead of reverting this. Let me know if I’m missing something.
Our extraction breaks because we don't recognize that some data represents a FID. If Farcaster introduces a new place where FID is present, nothing will happen - we won't notice this FID and won't fetch a corresponding user. How else could we handle this? Should we search the response with a regex and treat every number as a potential FID?
The only real problem is when Farcaster stops sending FIDs in one of the properties. We also can't handle it well, but at least we should notice it because the validation should fail.
It's also possible that I'm missing something fundamental - please correct me in that case.
We have an object that looks like this:
{ "conversationId": "4282-7811", "hasMention": false, "isDeleted": false, "isPinned": false, "isProgrammatic": false, "mentions": [], "message": "thanks again for helping test, your report was super helpful to getting the problem solved :)", "messageId": "08ac07f52d1c2ad9759ffea13edd695b", "reactions": [], "senderContext": { "displayName": "Ashoat", "fid": 7811, "pfp": { "url": "https://i.imgur.com/ToR9Mqd.jpg", "verified": false }, "username": "ashoat.eth" }, "senderFid": 7811, "serverTimestamp": 1761148247480, "type": "text", "viewerContext": { "focused": false, "isLastReadMessage": false, "reactions": [] } }
And a validator that looks like this:
const farcasterMessageValidator: TInterface<FarcasterMessage> = tShapeInexact({ conversationId: t.String, senderFid: tFarcasterID, messageId: t.String, serverTimestamp: t.Number, type: t.enums.of([ 'text', 'group_name_change', 'group_membership_addition', 'group_membership_removal', 'pin_message', 'message_ttl_change', 'rich_announcement', ]), message: t.String, reactions: t.list(farcasterReactionValidator), metadata: t.maybe(farcasterMessageMetadataValidator), viewerContext: t.maybe(farcasterMessageViewerContextValidator), isPinned: t.Boolean, isDeleted: t.Boolean, actionTargetUserContext: t.maybe(farcasterMessageUserContextValidator), });
Why isn't the root senderFid being picked up? extractFarcasterIDsFromPayload returns an empty array for these.
Here's a failing unit test: https://linear.app/comm/issue/ENG-11509/messages-appear-as-if-from-wrong-user#comment-94575408
D15516 fixes the issue I was referring to.
The only other question on this diff is whether it reintroduces FIDs that were previously missing and should have been picked up. I don't think we need senderContext, since there is already senderFid. Do we need mentions?
I think that if we don't use a property, we don't need to get the user mentioned in that property. It could happen that it references a user we don't have in our store, but it shouldn't matter because such a user won't be displayed in any significant way. If we start using the property value for something, then we should introduce a validator for it, and thus, the user will be added to the store.
So, using mentions as an example, we currently do nothing with them, so even if an unknown user is mentioned, it doesn't matter. When we start using the mentions for something, we will add a corresponding validator, and the user will be added.