Repository navigation
fix(slack): let healthy link previews survive a failed video lookup - #3363
Open
vedanth-jadhav wants to merge 5 commits into
Open
vedanth-jadhav wants to merge 5 commits into
vedanth-jadhav wants to merge 5 commits into
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Author
|
@greptile review Please review current head |
Author
|
@greptile review Re-review latest commit |
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem and trigger
A Slack
link_sharedevent can contain up to five Cap links. The handler resolves their videos concurrently withPromise.all, which rejects the entire batch if one lookup throws. That drops valid previews from the same message even when other videos were resolved successfully.Fix
Use
Promise.allSettledfor the independent per-video lookups and include only fulfilled, non-null results in the Slack unfurl map. Keep the existing five-link cap, deduplication, public visibility checks, stable link ordering, and one finalsendSlackUnfurlcall. Slack API and installation-token failures are still allowed to propagate; only a failed video lookup is isolated.Regression tests
Validation
main.bun run --cwd apps/web vitest run __tests__/unit/slack-unfurl.test.ts.

Confidence Score: 5/5
The PR appears safe to merge; no new actionable issues were found.Summary
This PR keeps healthy Slack previews when another video lookup fails.
Review used code inspection. Tests were not run.
Reviews (2) · Last reviewed commit: "style(slack): match Biome formatting for..." · Reviewed by Greptile