feat/enhanced-follow-list #85
Conversation
![callstackai[bot]](https://avatars.githubusercontent.com/in/1040204?s=60&v=4){kind=small}
There was a problem hiding this comment.
Key Issues
The code lacks proper error handling, with empty catch blocks and missing try-catch wrappers for async operations, risking unhandled errors and application crashes. Unsafe type assertions and null handling could lead to runtime errors if the response structure is unexpected. There is a logic error in data labeling, where Twitch data is incorrectly marked as Kick data, potentially causing data misinterpretation.
| try { | ||
| const kick = await this.commonUtils().getAssetFile(this.workerService(), "brands/kick.svg"); | ||
| this.platformIcons = { kick }; | ||
| } catch {} |
There was a problem hiding this comment.
🐛 Possible Bug
The empty catch block silently ignores any errors that occur while loading platform icons. This makes it difficult to diagnose issues if the icon fails to load. The error should be logged to help with debugging.
| } catch {} | |
| } catch (error) { this.logger.warn('Failed to load Kick platform icon', error); } |
| await this.loadStreamersFromCommon(); | ||
| await this.refreshStatuses(); |
There was a problem hiding this comment.
🐛 Possible Bug
The calls to loadStreamersFromCommon() and refreshStatuses() are not wrapped in try-catch blocks. If these async operations fail, they will result in unhandled promise rejections that could crash the application.
| await this.loadStreamersFromCommon(); | |
| await this.refreshStatuses(); | |
| try { | |
| await this.loadStreamersFromCommon(); | |
| await this.refreshStatuses(); | |
| } catch (error) { | |
| this.logger.error('Failed to initialize Kick streamers module', error); | |
| } |
| private async loadStreamersFromCommon(): Promise<void> { | ||
| try { | ||
| const res = await this.workerService().send("getCommon", { platform: "twitch", key: "kickStreamers" }); | ||
| const value = (res && (res as { value: unknown | null }).value) as unknown; |
There was a problem hiding this comment.
🐛 Possible Bug
The type assertion and null handling for the response from workerService().send() is unsafe. The code assumes the response will have a value property without proper validation. If res is null or doesn't match the expected structure, the type assertion could cause runtime errors.
| const value = (res && (res as { value: unknown | null }).value) as unknown; | |
| const value = res?.value ?? null; |
| await this.workerService().send("setCommon", { | ||
| platform: "kick", | ||
| key: COMMON_KEYS.kick.twitchStreamers, | ||
| value: twitchFollowList, | ||
| }); |
There was a problem hiding this comment.
🐛 Possible Bug
The code is storing Twitch follow list data but setting the platform as 'kick'. This appears to be a logic error as the data source is from Twitch but is being labeled as Kick data.
| await this.workerService().send("setCommon", { | |
| platform: "kick", | |
| key: COMMON_KEYS.kick.twitchStreamers, | |
| value: twitchFollowList, | |
| }); | |
| await this.workerService().send("setCommon", { | |
| platform: "twitch", | |
| key: COMMON_KEYS.kick.twitchStreamers, | |
| value: twitchFollowList, | |
| }); |
| headers: { Authorization: authorization }, | ||
| credentials: "include", | ||
| }); | ||
| if (!res.ok) break; |
There was a problem hiding this comment.
🐛 Possible Bug
The code breaks the loop on any non-OK response (!res.ok) without checking the specific error. This could mask important API errors like rate limiting or auth issues, leading to incomplete data collection.
| if (!res.ok) break; | |
| if (!res.ok) throw new Error(`API request failed with status ${res.status}`); |
There was a problem hiding this comment.
Key Issues
The PR review highlights critical issues including the failure to create necessary database indexes during version upgrades, leading to an invalid database state; unchecked access to potentially undefined properties causing runtime errors; and a performance regression due to sequential data fetching instead of concurrent requests.
Co-authored-by: callstackai[bot] <186726322+callstackai[bot]@users.noreply.github.com>
Co-authored-by: callstackai[bot] <186726322+callstackai[bot]@users.noreply.github.com>
Description
Briefly explain what this PR does. Is it a bug fix, new feature, or a refactor?
Testing
Select all the environments you tested this PR with:
Please describe how you tested this change in the selected environments.
Related Issues
If this PR addresses an issue, link it here (e.g.,
Closes #123).✨
Description by Callstackai
This PR introduces a new feature that allows sharing followed channels across platforms, specifically between Twitch and Kick. It includes the implementation of follow synchronization and settings for enabling/disabling this feature.
Diagrams of code changes
sequenceDiagram participant User participant Platform participant SharedFollowsModule participant FollowSyncer participant CommonDataService participant WorkerService participant CommonDatabase User->>Platform: Enable follow sharing Platform->>SharedFollowsModule: Initialize module SharedFollowsModule->>FollowSyncer: Start sync timer loop Every few minutes FollowSyncer->>Platform: Get followed channels Platform-->>FollowSyncer: Return channel list FollowSyncer->>CommonDataService: Update shared follows CommonDataService->>WorkerService: Send update request WorkerService->>CommonDatabase: Store follows data CommonDatabase-->>WorkerService: Confirm storage WorkerService-->>CommonDataService: Return success end Note over Platform,CommonDatabase: Follows are now shared between platforms User->>Platform: View follows from other platform Platform->>CommonDataService: Request shared follows CommonDataService->>WorkerService: Get common data WorkerService->>CommonDatabase: Retrieve follows CommonDatabase-->>WorkerService: Return follows data WorkerService-->>CommonDataService: Return data CommonDataService-->>Platform: Return follows list Platform-->>User: Display combined followsFiles Changed