Skip to content

fix: playlists containing track error#86

Merged
roitium merged 1 commit into
devfrom
hotfix/fix-playlists-containing-track
Nov 9, 2025
Merged

fix: playlists containing track error#86
roitium merged 1 commit into
devfrom
hotfix/fix-playlists-containing-track

Conversation

@roitium
Copy link
Copy Markdown
Collaborator

@roitium roitium commented Nov 9, 2025

Summary by CodeRabbit

  • Refactor
    • Streamlined internal service implementations across playlist, track, download, and lyric operations to improve code maintainability and reliability. Simplified request handling and error management patterns for enhanced consistency across core services. No changes to user-facing functionality.

@roitium roitium added the bug Something isn't working label Nov 9, 2025
@safedep
Copy link
Copy Markdown

safedep Bot commented Nov 9, 2025

SafeDep Report Summary

Green Malicious Packages Badge Green Vulnerable Packages Badge Green Risky License Badge

No dependency changes detected. Nothing to scan.

This report is generated by SafeDep Github App

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 9, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request removes Sentry tracing instrumentation from multiple facade and service layers, replacing nested span wrappers with direct asynchronous control flow using ResultAsync patterns. The changes affect nine core files including sync, playlist, and track management services while preserving existing functionality and error handling semantics.

Changes

Cohort / File(s) Summary
Facade Layer Tracing Removal
src/lib/facades/bilibili.ts, src/lib/facades/playlist.ts, src/lib/facades/sync.ts
Removed Sentry startSpan wrappers from all facade methods (fetchRemotePlaylistMetadata, duplicatePlaylist, updateTrackLocalPlaylists, batchAddTracksToLocalPlaylist, addTrackFromBilibiliApi, syncCollection, syncMultiPageVideo, syncFavorite). Flattened nested async callbacks into linear sequences while maintaining transaction boundaries, error handling, and return values.
Artist Service Refactoring
src/lib/services/artistService.ts
Replaced nested Sentry span structures with ResultAsync.fromPromise across all methods. Moved input validation outside wrapped blocks. Consolidated error handling to standardize DatabaseError mapping. Updated findOrCreateManyRemoteArtists with early validation and bulk insert flow.
Download Service Cleanup
src/lib/services/downloadService.ts
Removed Sentry span wrappers from getDownloadUrl, start, delete, and deleteAll. Replaced nested spans with direct try/catch and promise chains. Inlined progress reporting and file operations while preserving HTTP fetch, stream piping, and state update logic.
Lyric Service Simplification
src/lib/services/lyricService.ts
Removed Sentry spans from seven methods (getBestMatchedLyrics, smartFetchLyrics, migrateFromOldFormat, getPreciseMusicNameOnBilibiliVideo, clearAllLyrics, saveLyricsToFile, fetchLyrics). Replaced with direct try/catch flows and ResultAsync patterns. Linearized cache and bilibili-specific logic paths.
Playlist Service Restructuring
src/lib/services/playlistService.ts
Removed nested Sentry spans across fourteen methods. Flattened all operations into single async blocks via ResultAsync.fromPromise. Simplified reordering, pagination, and batch operations. Added public export: export const playlistService = new PlaylistService(db, trackService).
Track Service Optimization
src/lib/services/trackService.ts
Replaced nested Sentry spans with direct execution in create, find, and leaderboard methods. Consolidated _createTrack transaction logic. Refactored findOrCreateManyTracks with batch preprocessing and source validation. Flattened cursor-based pagination and duration calculations.
Update Service Streamlining
src/lib/services/updateService.ts
Removed outer Sentry span wrapper from fetchLatestRelease and checkForAppUpdate. Added early return for missing manifest URL. Simplified to direct try/catch flow for fetch, parse, and validation steps.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SyncFacade
    participant DB
    participant TrackService
    
    rect rgb(200, 220, 255)
    Note over SyncFacade: Before: Nested Sentry Spans
    Client->>SyncFacade: sync(remoteSyncId, type)
    activate SyncFacade
    SyncFacade->>SyncFacade: Sentry.startSpan (outer)
    SyncFacade->>SyncFacade: Sentry.startSpan (inner)
    SyncFacade->>DB: query/insert operations
    SyncFacade->>TrackService: operations
    SyncFacade->>SyncFacade: span.end()
    SyncFacade->>SyncFacade: span.end()
    deactivate SyncFacade
    end
    
    rect rgb(220, 255, 220)
    Note over SyncFacade: After: Direct Flow
    Client->>SyncFacade: sync(remoteSyncId, type)
    activate SyncFacade
    SyncFacade->>SyncFacade: switch on type
    SyncFacade->>DB: query/insert operations
    SyncFacade->>TrackService: operations
    SyncFacade->>SyncFacade: try/catch error handling
    deactivate SyncFacade
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • src/lib/facades/sync.ts — Major refactoring of syncFavorite with multi-step diff-based synchronization flow; requires verification of remote/local track diffing and ordered track ID computation
  • src/lib/services/trackService.ts — Complex changes to batch operations (findOrCreateManyTracks) with preprocessing validation and bulk insert logic; verify bilibili metadata insertion and cursor handling
  • src/lib/services/playlistService.ts — Extensive flattening across 14+ methods; verify all query conditions, transaction boundaries, and pagination cursor mechanics (nextCursor computation)
  • Error propagation — All error paths converted to ResultAsync chains; verify no errors are swallowed and DatabaseError/ServiceError distinctions are preserved
  • Public API change — New export of playlistService singleton at end of playlistService.ts; verify this doesn't create circular dependencies or unintended side effects

Possibly related PRs

Poem

🐰 Spans be gone, the traces fade,
Linear flows in light parade,
Async chains now crystal clear,
Sentry's whispers disappear,
ResultAsync hops along the way—
Code runs swift from night to day! 🌙✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix/fix-playlists-containing-track

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 292c06c and 31fc795.

📒 Files selected for processing (9)
  • src/lib/facades/bilibili.ts (1 hunks)
  • src/lib/facades/playlist.ts (3 hunks)
  • src/lib/facades/sync.ts (5 hunks)
  • src/lib/services/artistService.ts (8 hunks)
  • src/lib/services/downloadService.ts (3 hunks)
  • src/lib/services/lyricService.ts (2 hunks)
  • src/lib/services/playlistService.ts (18 hunks)
  • src/lib/services/trackService.ts (12 hunks)
  • src/lib/services/updateService.ts (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@roitium roitium merged commit 6301374 into dev Nov 9, 2025
3 of 4 checks passed
roitium added a commit that referenced this pull request Dec 1, 2025
fix: playlists containing track error (#86)
@roitium roitium deleted the hotfix/fix-playlists-containing-track branch December 14, 2025 07:13
@coderabbitai coderabbitai Bot mentioned this pull request Feb 27, 2026
@coderabbitai coderabbitai Bot mentioned this pull request May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant