Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccou
import { getAccountArtistIds } from "@/lib/supabase/account_artist_ids/getAccountArtistIds";
import selectAccountEmails from "@/lib/supabase/account_emails/selectAccountEmails";
import { uploadLinkToArweave } from "@/lib/arweave/uploadLinkToArweave";
import { filterNewPostUrls } from "@/lib/socials/filterNewPostUrls";

vi.mock("@/lib/apify/client", () => ({ default: { dataset: vi.fn() } }));

Expand All @@ -26,6 +27,7 @@ vi.mock("../handleInstagramProfileFollowUpRuns", () => ({
handleInstagramProfileFollowUpRuns: vi.fn(),
}));
vi.mock("@/lib/apify/sendApifyWebhookEmail", () => ({ sendApifyWebhookEmail: vi.fn() }));
vi.mock("@/lib/socials/filterNewPostUrls", () => ({ filterNewPostUrls: vi.fn() }));
vi.mock("@/lib/supabase/socials/upsertSocials", () => ({ upsertSocials: vi.fn() }));
vi.mock("@/lib/supabase/socials/selectSocials", () => ({
selectSocials: vi.fn(),
Expand All @@ -51,7 +53,11 @@ const payload = {
} as never;

describe("handleInstagramProfileScraperResults", () => {
beforeEach(() => vi.clearAllMocks());
beforeEach(() => {
vi.clearAllMocks();
// default: everything scraped counts as new (per-test overrides below)
vi.mocked(filterNewPostUrls).mockImplementation(async urls => urls);
});

it("short-circuits when the dataset has no latest posts", async () => {
mockDataset([{ username: "alice" }]);
Expand Down Expand Up @@ -92,4 +98,34 @@ describe("handleInstagramProfileScraperResults", () => {
expect(result.social).toEqual({ id: "s1" });
expect(result.posts).toEqual(posts);
});

it("skips the alert email when no scraped post is genuinely new (chat#1855)", async () => {
mockDataset([
{
latestPosts: [{ url: "u1", timestamp: "t" }],
username: "alice",
url: "instagram.com/alice",
profilePicUrl: "https://a",
fullName: "Alice",
},
]);
vi.mocked(filterNewPostUrls).mockResolvedValue([]); // all already stored
vi.mocked(upsertPosts).mockResolvedValue({ data: null, error: null } as never);
vi.mocked(getPosts).mockResolvedValue([{ id: "p1", post_url: "u1" }] as never);
vi.mocked(uploadLinkToArweave).mockResolvedValue(null);
vi.mocked(upsertSocials).mockResolvedValue([] as never);
vi.mocked(selectSocials).mockResolvedValue([{ id: "s1" }] as never);
vi.mocked(selectAccountSocials).mockResolvedValue([{ account_id: "a1" }] as never);
vi.mocked(getAccountArtistIds).mockResolvedValue([{ account_id: "a1" }] as never);
vi.mocked(selectAccountEmails).mockResolvedValue([{ email: "x@y.com" }] as never);

const result = await handleInstagramProfileScraperResults(payload);

expect(sendApifyWebhookEmail).not.toHaveBeenCalled();
// persistence is unaffected — only the notification is gated
expect(upsertPosts).toHaveBeenCalledOnce();
expect(upsertSocialPosts).toHaveBeenCalledOnce();
expect(handleInstagramProfileFollowUpRuns).toHaveBeenCalledOnce();
expect(result.social).toEqual({ id: "s1" });
});
});
16 changes: 12 additions & 4 deletions lib/apify/instagram/handleInstagramProfileScraperResults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { uploadLinkToArweave } from "@/lib/arweave/uploadLinkToArweave";
import { getFetchableUrl } from "@/lib/arweave/getFetchableUrl";
import type { ApifyInstagramProfileResult } from "@/lib/apify/types";
import type { ApifyWebhookPayload } from "@/lib/apify/validateApifyWebhookRequest";
import { filterNewPostUrls } from "@/lib/socials/filterNewPostUrls";
import type { TablesInsert } from "@/types/database.types";

/**
Expand All @@ -40,6 +41,9 @@ export async function handleInstagramProfileScraperResults(parsed: ApifyWebhookP
post.url ? [{ post_url: post.url, updated_at: post.timestamp }] : [],
);
if (postRows.length === 0) return { posts: [], social: null };
// Diff BEFORE upserting — afterwards every scraped post exists and nothing
// is distinguishable as new (chat#1855).
const newPostUrls = await filterNewPostUrls(postRows.map(p => p.post_url));
await upsertPosts(postRows);
Comment on lines +44 to 47

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Unguarded filterNewPostUrls call can block post persistence.

filterNewPostUrls runs before upsertPosts. If the DB read inside getPosts throws (transient network/DB error), upsertPosts never executes and scraped posts are lost. Previously, persistence was unconditional — the email logic came after. This introduces a new failure mode where a read error blocks the write.

Wrap the diff in a try-catch and default to all URLs being "new" on failure, so persistence proceeds and the worst case is a duplicate email (the pre-PR behavior).

🛡️ Proposed fix: degrade gracefully on diff failure
   // Diff BEFORE upserting — afterwards every scraped post exists and nothing
   // is distinguishable as new (chat#1855).
-  const newPostUrls = await filterNewPostUrls(postRows.map(p => p.post_url));
+  let newPostUrls: string[];
+  try {
+    newPostUrls = await filterNewPostUrls(postRows.map(p => p.post_url));
+  } catch (error) {
+    console.error("[WARN] filterNewPostUrls failed, assuming all posts are new:", error);
+    newPostUrls = postRows.map(p => p.post_url);
+  }
   await upsertPosts(postRows);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Diff BEFORE upserting — afterwards every scraped post exists and nothing
// is distinguishable as new (chat#1855).
const newPostUrls = await filterNewPostUrls(postRows.map(p => p.post_url));
await upsertPosts(postRows);
// Diff BEFORE upserting — afterwards every scraped post exists and nothing
// is distinguishable as new (chat#1855).
let newPostUrls: string[];
try {
newPostUrls = await filterNewPostUrls(postRows.map(p => p.post_url));
} catch (error) {
console.error("[WARN] filterNewPostUrls failed, assuming all posts are new:", error);
newPostUrls = postRows.map(p => p.post_url);
}
await upsertPosts(postRows);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/apify/instagram/handleInstagramProfileScraperResults.ts` around lines 44
- 47, The new pre-upsert read in handleInstagramProfileScraperResults can
prevent persistence if filterNewPostUrls (and its underlying getPosts read)
throws, so update this flow to fail open. Wrap the filterNewPostUrls call in a
try-catch inside handleInstagramProfileScraperResults, and on any read/error
path default newPostUrls to all post_urls from postRows so upsertPosts still
runs unconditionally. Keep the existing postRows handling intact and ensure the
fallback preserves the prior behavior where write success is not blocked by read
failures.

const posts = await getPosts({ postUrls: postRows.map(p => p.post_url) });

Expand Down Expand Up @@ -96,10 +100,14 @@ export async function handleInstagramProfileScraperResults(parsed: ApifyWebhookP
// mail outage doesn't block comment scraping and vice versa.
let sentEmails = null;
try {
sentEmails = await sendApifyWebhookEmail(
firstResult,
accountEmails.map(e => e.email).filter(Boolean),
);
// Only notify when the scrape actually found posts new to the platform —
// otherwise every scrape re-announces the profile's recent feed.
if (newPostUrls.length > 0) {
sentEmails = await sendApifyWebhookEmail(
firstResult,
accountEmails.map(e => e.email).filter(Boolean),
);
}
} catch (error) {
console.error("[WARN] webhook email failed:", error);
}
Expand Down
25 changes: 25 additions & 0 deletions lib/socials/__tests__/filterNewPostUrls.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { filterNewPostUrls } from "@/lib/socials/filterNewPostUrls";
import { getPosts } from "@/lib/supabase/posts/getPosts";

vi.mock("@/lib/supabase/posts/getPosts", () => ({ getPosts: vi.fn() }));

beforeEach(() => vi.clearAllMocks());

describe("filterNewPostUrls", () => {
it("returns only URLs not already stored in posts", async () => {
vi.mocked(getPosts).mockResolvedValue([{ post_url: "u1" }, { post_url: "u3" }] as never);
expect(await filterNewPostUrls(["u1", "u2", "u3", "u4"])).toEqual(["u2", "u4"]);
expect(getPosts).toHaveBeenCalledWith({ postUrls: ["u1", "u2", "u3", "u4"] });
});

it("returns every URL when none are stored", async () => {
vi.mocked(getPosts).mockResolvedValue([] as never);
expect(await filterNewPostUrls(["u1"])).toEqual(["u1"]);
});

it("returns [] for empty input without querying", async () => {
expect(await filterNewPostUrls([])).toEqual([]);
expect(getPosts).not.toHaveBeenCalled();
});
});
21 changes: 21 additions & 0 deletions lib/socials/filterNewPostUrls.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { getPosts } from "@/lib/supabase/posts/getPosts";

/**
* Returns the subset of post URLs that are genuinely new to the platform —
* i.e. not already present in `posts`. Must run BEFORE the scrape results are
* upserted (afterwards everything exists and nothing is "new").
*
* Used to gate scrape-alert notifications: a scrape re-reads a profile's
* whole recent feed, so without this diff every scrape re-announces old
* posts as new (recoupable/chat#1855).
*
* @param postUrls - Candidate post URLs from a scrape result.
* @returns URLs with no existing `posts` row.
*/
export async function filterNewPostUrls(postUrls: string[]): Promise<string[]> {
if (!postUrls.length) return [];

const existing = await getPosts({ postUrls });
const known = new Set((existing ?? []).map(post => post.post_url));
return postUrls.filter(url => !known.has(url));
}
Loading