fix: distinguish cookie DB lock from fresh login cookie errors#46
fix: distinguish cookie DB lock from fresh login cookie errors#46TTAWDTT wants to merge 3 commits intovanloctech:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f721bfeca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/lib/cookie-error.ts
Outdated
| errorMessage?: string, | ||
| ): CookieErrorType | null { | ||
| const resolvedCode = | ||
| errorCode ?? (errorMessage ? inferBackendErrorCode(errorMessage) : undefined); |
There was a problem hiding this comment.
Use message fallback for non-cookie backend codes
inferCookieErrorType currently uses errorCode ?? inferBackendErrorCode(errorMessage), so any present backend code (even unrelated ones) prevents message-based cookie detection. In the download flow we emit error_code for yt-dlp failures (src-tauri/src/commands/download.rs, around line 913), and backend inference still only tags one cookie-lock pattern (src-tauri/src/types/error.rs, lines 161-170), so DPAPI/permission-denied cookie extraction errors now arrive with generic codes and no longer open the cookie remediation dialog that the previous regex-based logic handled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90674bb.
I kept the cookie-specific backend codes as the fast path, but unrelated/generic backend codes no longer suppress message-based cookie detection. We now still fall back to the error message for cookie-specific patterns, so DPAPI / permission-denied cookie extraction failures continue to open the remediation dialog.
I also added a regression test covering the generic-code + DPAPI/cookie-message case.
vanloctech
left a comment
There was a problem hiding this comment.
One additional finding is outside the changed diff, so I could not place it inline:
src-tauri/src/services/ytdlp.rs:parse_ytdlp_errorstill misses the newfresh login cookieswording.infer_error_codewas updated, butrun_ytdlp_jsonand transcript flows callparse_ytdlp_errorfirst; when stderr saysrequires fresh login cookies, this branch does not match and those flows fall back to a genericyt-dlp command failed. Please mirror the new detection here and add a regression test forparse_ytdlp_error.
| <> | ||
| <p className="font-medium text-foreground">Authentication needed</p> | ||
| <p className="text-muted-foreground mt-1"> | ||
| This content requires login cookies to download. Enable cookie mode in Settings |
There was a problem hiding this comment.
fresh_required here only tells users to enable cookie mode, but this error means the current auth cookies are stale. If cookie mode is already enabled, this sends them into a retry loop without telling them to refresh browser login or replace the cookie file. Please make the recovery steps explicitly say re-login in the browser or export a fresh cookie file.
|
Thanks for pointing this out! I have updated parse_ytdlp_error in src-tauri/src/services/ytdlp.rs to mirror the fresh login cookies detection logic from infer_error_code (including the new 'requires fresh login cookies' wording and the Douyin + cookies + fresh + needed/requires combination). I also added a regression test parse_ytdlp_error_detects_fresh_login_cookies_phrase in src-tauri/tests/ytdlp_parse_error_tests.rs that asserts we return the YT_FRESH_COOKIES_REQUIRED code and the specific guidance message for that stderr phrase. |
Summary
Validation
Notes