refactor: Add default timeout for thumbnail downloads

This commit is contained in:
Peifan Li
2026-01-04 22:03:41 -05:00
parent 695489d72a
commit 494b85d440
5 changed files with 119 additions and 26 deletions

View File

@@ -41,6 +41,9 @@ export abstract class BaseDownloader implements IDownloader {
options?: DownloadOptions
): Promise<Video>;
// Default timeout for thumbnail downloads (60 seconds)
protected static readonly THUMBNAIL_DOWNLOAD_TIMEOUT = 60000;
/**
* Common helper to download a thumbnail
*/
@@ -51,7 +54,7 @@ export abstract class BaseDownloader implements IDownloader {
): Promise<boolean> {
try {
logger.info("Downloading thumbnail from:", thumbnailUrl);
if (axiosConfig.proxy) {
if (axiosConfig.proxy || axiosConfig.httpAgent) {
logger.debug("Using proxy for thumbnail download");
}
@@ -62,6 +65,7 @@ export abstract class BaseDownloader implements IDownloader {
method: "GET",
url: thumbnailUrl,
responseType: "stream",
timeout: BaseDownloader.THUMBNAIL_DOWNLOAD_TIMEOUT,
...axiosConfig,
});

View File

@@ -13,6 +13,7 @@ import {
getAxiosProxyConfig,
getNetworkConfigFromUserConfig,
getUserYtDlpConfig,
InvalidProxyError,
} from "../../utils/ytDlpUtils";
import * as storageService from "../storageService";
import { Video } from "../storageService";
@@ -163,13 +164,13 @@ export class MissAVDownloader extends BaseDownloader {
// 4. Select the best m3u8 URL from collected URLs
let m3u8Url = MissAVDownloader.selectBestM3u8Url(m3u8Urls, hasFormatSort);
if (m3u8Url) {
logger.info(
`Selected m3u8 URL from ${m3u8Urls.length} candidates (format sort: ${hasFormatSort}):`,
m3u8Url
);
const alternatives = m3u8Urls.filter(u => u !== m3u8Url);
const alternatives = m3u8Urls.filter((u) => u !== m3u8Url);
if (alternatives.length > 0) {
logger.info("Alternative URLs:", alternatives);
}
@@ -328,11 +329,7 @@ export class MissAVDownloader extends BaseDownloader {
const args = [m3u8Url, ...flagsToArgs(flags)];
// Log the full command for debugging
logger.info(
"Executing yt-dlp command:",
YT_DLP_PATH,
args.join(" ")
);
logger.info("Executing yt-dlp command:", YT_DLP_PATH, args.join(" "));
try {
await new Promise<void>((resolve, reject) => {
@@ -393,9 +390,21 @@ export class MissAVDownloader extends BaseDownloader {
// 8. Download and save the thumbnail
if (thumbnailUrl) {
// Use base class method via temporary instance
const axiosConfig = userConfig.proxy
? getAxiosProxyConfig(userConfig.proxy)
: {};
let axiosConfig = {};
if (userConfig.proxy) {
try {
axiosConfig = getAxiosProxyConfig(userConfig.proxy);
} catch (error) {
if (error instanceof InvalidProxyError) {
logger.warn(
"Invalid proxy configuration for thumbnail download, proceeding without proxy:",
error.message
);
} else {
throw error;
}
}
}
const downloader = new MissAVDownloader();
thumbnailSaved = await downloader.downloadThumbnail(
thumbnailUrl,
@@ -509,7 +518,10 @@ export class MissAVDownloader extends BaseDownloader {
}
// Helper to select best m3u8 URL
static selectBestM3u8Url(urls: string[], hasFormatSort: boolean): string | null {
static selectBestM3u8Url(
urls: string[],
hasFormatSort: boolean
): string | null {
if (urls.length === 0) return null;
const sortedUrls = [...urls].sort((a, b) => {
@@ -543,10 +555,8 @@ export class MissAVDownloader extends BaseDownloader {
// BUT, given the bug report where a 240p stream was picked over a master,
// we should probably trust the master playlist more particularly if the alternative is low quality.
// However, if we have a high quality specific stream (e.g. 720p/1080p explicit), that might be fine.
// Let's refine: If one is surrit master, pick it. (Handled by step 1 & surrit sub-logic)
// If neither is surrit, and one is master...
// If both are master or both are not master, compare resolution.
}
@@ -559,7 +569,7 @@ export class MissAVDownloader extends BaseDownloader {
// If we have a significant resolution difference, we might prefer the higher one
// UNLESS one is a master playlist and the other is a low res specific one.
// If one is master (0p detected) and other is 240p, 0p (master) should win if it's likely to contain better streams.
// Updated Strategy:
// If both have resolution, compare them.
if (aQualityNum > 0 && bQualityNum > 0) {
@@ -570,14 +580,14 @@ export class MissAVDownloader extends BaseDownloader {
// If we are prioritizing master playlists (e.g. because of surrit or format sort), master wins.
// If we are NOT specifically prioritizing master, we still might want to prefer it over very low res (e.g. < 480p).
if (aIsMaster && bQualityNum > 0 && bQualityNum < 480) return -1; // Master wins over < 480p
if (bIsMaster && aQualityNum > 0 && aQualityNum < 480) return 1; // Master wins over < 480p
if (bIsMaster && aQualityNum > 0 && aQualityNum < 480) return 1; // Master wins over < 480p
// Fallback: Default to higher number (so 720p wins over 0p/master if we didn't catch it above)
// This preserves 'best attempt' for specific high quality URLs if they exist not on surrit.
if (aQualityNum !== bQualityNum) {
return bQualityNum - aQualityNum;
}
// Final tie-breaker: prefer master if all else equal
if (aIsMaster && !bIsMaster) return -1;
if (!aIsMaster && bIsMaster) return 1;

View File

@@ -12,6 +12,7 @@ import {
getAxiosProxyConfig,
getNetworkConfigFromUserConfig,
getUserYtDlpConfig,
InvalidProxyError,
} from "../../../utils/ytDlpUtils";
import * as storageService from "../../storageService";
import { Video } from "../../storageService";
@@ -258,9 +259,21 @@ export async function downloadVideo(
let thumbnailSaved = false;
if (thumbnailUrl) {
// Use base class method via temporary instance
const axiosConfig = userConfig.proxy
? getAxiosProxyConfig(userConfig.proxy)
: {};
let axiosConfig = {};
if (userConfig.proxy) {
try {
axiosConfig = getAxiosProxyConfig(userConfig.proxy);
} catch (error) {
if (error instanceof InvalidProxyError) {
logger.warn(
"Invalid proxy configuration for thumbnail download, proceeding without proxy:",
error.message
);
} else {
throw error;
}
}
}
const downloader = new BilibiliDownloaderHelper();
thumbnailSaved = await downloader.downloadThumbnailPublic(
thumbnailUrl,
@@ -445,9 +458,21 @@ export async function downloadSinglePart(
: collectionName
? `/subtitles/${collectionName}`
: `/subtitles`;
const axiosConfig = userConfig.proxy
? getAxiosProxyConfig(userConfig.proxy)
: {};
let axiosConfig = {};
if (userConfig.proxy) {
try {
axiosConfig = getAxiosProxyConfig(userConfig.proxy);
} catch (error) {
if (error instanceof InvalidProxyError) {
logger.warn(
"Invalid proxy configuration for subtitle download, proceeding without proxy:",
error.message
);
} else {
throw error;
}
}
}
subtitles = await downloadSubtitles(
url,
newSafeBaseFilename,

View File

@@ -14,6 +14,7 @@ import {
getAxiosProxyConfig,
getNetworkConfigFromUserConfig,
getUserYtDlpConfig,
InvalidProxyError,
} from "../../../utils/ytDlpUtils";
import * as storageService from "../../storageService";
import { Video } from "../../storageService";
@@ -305,7 +306,20 @@ export async function downloadVideo(
let axiosConfig = {};
if (downloadUserConfig.proxy) {
axiosConfig = getAxiosProxyConfig(downloadUserConfig.proxy);
try {
axiosConfig = getAxiosProxyConfig(downloadUserConfig.proxy);
} catch (error) {
if (error instanceof InvalidProxyError) {
// Log the error but continue without proxy for thumbnail
// Video download already succeeded, don't fail for thumbnail proxy issues
logger.warn(
"Invalid proxy configuration for thumbnail download, proceeding without proxy:",
error.message
);
} else {
throw error;
}
}
}
thumbnailSaved = await downloader.downloadThumbnailPublic(

View File

@@ -511,10 +511,28 @@ export function getNetworkConfigFromUserConfig(
return networkOptions;
}
/**
* Error thrown when proxy configuration is invalid
*/
export class InvalidProxyError extends Error {
readonly proxyUrl: string;
readonly originalError?: Error;
constructor(proxyUrl: string, originalError?: Error) {
super(`Invalid proxy URL: ${proxyUrl}`);
this.name = "InvalidProxyError";
this.proxyUrl = proxyUrl;
this.originalError = originalError;
}
}
/**
* Helper to convert a proxy URL string into an Axios config object
* Supports http/https/socks5 proxies with authentication
* Format: http://user:pass@host:port or socks5://user:pass@host:port
*
* @throws {InvalidProxyError} If the proxy URL is malformed - this prevents
* silent fallback to direct connection which could expose user's real IP
*/
export function getAxiosProxyConfig(proxyUrl: string): any {
if (!proxyUrl) return {};
@@ -523,6 +541,11 @@ export function getAxiosProxyConfig(proxyUrl: string): any {
const url = new URL(proxyUrl);
const protocol = url.protocol.replace(":", "");
// Validate that we have a hostname
if (!url.hostname) {
throw new InvalidProxyError(proxyUrl, new Error("Missing hostname"));
}
// Check if this is a SOCKS proxy
if (protocol.startsWith("socks")) {
// Use SocksProxyAgent for SOCKS proxy support
@@ -534,6 +557,14 @@ export function getAxiosProxyConfig(proxyUrl: string): any {
};
}
// Validate protocol for non-SOCKS proxies
if (protocol !== "http" && protocol !== "https") {
throw new InvalidProxyError(
proxyUrl,
new Error(`Unsupported proxy protocol: ${protocol}`)
);
}
// Handle HTTP/HTTPS proxies
const isHttps = protocol === "https";
const defaultPort = isHttps ? 443 : 80;
@@ -554,7 +585,16 @@ export function getAxiosProxyConfig(proxyUrl: string): any {
return { proxy: proxyConfig };
} catch (error) {
console.error("Invalid proxy URL:", proxyUrl);
return {};
// Re-throw InvalidProxyError as-is
if (error instanceof InvalidProxyError) {
throw error;
}
// Wrap other errors (like URL parsing errors) in InvalidProxyError
// This ensures we fail rather than silently falling back to direct connection
console.error("Invalid proxy URL:", proxyUrl, error);
throw new InvalidProxyError(
proxyUrl,
error instanceof Error ? error : new Error(String(error))
);
}
}