fix(youtube): stop swallowing channel-list errors in pages() - #2070
giladresisi wants to merge 1 commit into
Conversation
Let a failed channels.list propagate like the Facebook provider does, so the error reaches Sentry instead of being turned into an empty list. The continue-integration dialog already catches the failed request and shows the same "no channels found" state. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gsRv3MWm5YFweP58FM31H
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| return channels.map((channel) => ({ | ||
| id: channel.id!, | ||
| name: channel.snippet?.title || 'Unnamed Channel', | ||
| picture: { | ||
| data: { | ||
| url: channel.snippet?.thumbnails?.default?.url || '', | ||
| }, | ||
| username: channel.snippet?.customUrl || '', | ||
| subscriberCount: channel.statistics?.subscriberCount || '0', | ||
| })); | ||
| } catch (error) { | ||
| console.error('Failed to fetch YouTube channels:', error); | ||
| return []; | ||
| } | ||
| }, | ||
| username: channel.snippet?.customUrl || '', | ||
| subscriberCount: channel.statistics?.subscriberCount || '0', | ||
| })); |
There was a problem hiding this comment.
Bug: Removing the try/catch in pages() causes an unhandled exception in the reConnect() path during token refresh, as the calling refreshProcess() does not handle errors from reConnect().
Severity: MEDIUM
Suggested Fix
Wrap the call to integrationProvider.reConnect() within the refreshProcess() function in refresh.integration.service.ts with a try/catch block. This will handle potential failures from the pages() method during the token refresh workflow gracefully, preventing the process from crashing while allowing for proper error logging.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
libraries/nestjs-libraries/src/integrations/social/youtube.provider.ts#L341-L362
Potential issue: The removal of the `try/catch` block within the `pages()` method
introduces an unhandled exception in the token refresh workflow. While the primary
`connectSocialMedia` flow remains unaffected due to an existing controller-level
`try/catch`, the `reConnect()` method calls `pages()` without any error handling. The
caller, `refreshProcess()`, also lacks a `try/catch` around the `reConnect()` call.
Consequently, any API error from `youtubeClient.channels.list()` during a token refresh
will now cause the refresh workflow to crash, whereas it previously failed silently by
returning an empty array.
Also affects:
apps/backend/src/api/routes/no.auth.integrations.controller.ts:265~285
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Not a behavior change. Before this PR a failed channels.list made pages() return [], so reConnect() threw "Channel not found" at the same point in refreshProcess(). The refresh path already handled a throw there; it now gets the real Google error instead of a misleading one. The no-auth controller call site already has its own try/catch around pages().
This branch has not been deployed
1 participant
What kind of change does this PR introduce?
Observability fix (backend, YouTube provider).
YoutubeProvider.pages()wrappedchannels.listin a try/catch that logged to the console and returned an empty list. The try/catch is removed so the error propagates from thepagesfunction endpoint and is captured by the Nest Sentry integration, matchingFacebookProvider.pages(), which has no catch. The channel mapping and themine: truequery are unchanged. The frontend continue-integration dialog already catches a failed request in itsloadDataand renders the same "We couldn't find any YouTube channels" empty state, so the customer-facing behavior is identical.Why was this change needed?
A customer with several YouTube channels under one Google account connected one channel, then had two more attempts stay stuck at the channel-selection step. The second attempt reused the personal account and only re-offered the already-connected channel. The third attempt was a brand account and never got past selection. Its failure is unexplained: with the catch in place, a failed channel listing leaves nothing in Sentry or the DB, only a console line, so it cannot be distinguished from the user closing the dialog. Letting the throw propagate gives us the actual API error next time.
The alternative, keeping the catch and calling
Sentry.captureExceptioninside it, was not chosen: no provider calls Sentry directly today (the only direct use is a metrics counter in the posts service), so it would introduce a new pattern for the same result.Other information:
The "already connected channel offered again" case shows a heading with an empty grid in the shared continue dialog. That is a separate UX issue that overlaps with PR #1719 and is not addressed here.
QA
channels.listat an invalid part) and reopen the dialog.pagesfunction call instead of only a console line.Checklist:
🤖 Generated with Claude Code
https://claude.ai/code/session_017gsRv3MWm5YFweP58FM31H