diff --git a/src/api/controllers/map.controller.ts b/src/api/controllers/map.controller.ts index d7affd4..fbb89a7 100644 --- a/src/api/controllers/map.controller.ts +++ b/src/api/controllers/map.controller.ts @@ -73,17 +73,12 @@ export class MapController { crawlOptions }; - // Run URL discovery with timeout wrapper to prevent hanging - const discoveryTimeoutMs = timeoutMs ?? 60000; // Use user-provided timeout or default to 60 seconds - - const timeoutPromise = new Promise((_, reject) => - setTimeout(() => reject(new Error(`Discovery timeout after ${discoveryTimeoutMs}ms`)), discoveryTimeoutMs) - ); - - const discoveryResult = await Promise.race([ - this.discoveryService.discoverUrls(discoveryOptions), - timeoutPromise - ]); + // The discovery service now enforces `timeoutMs` internally as a SOFT + // deadline and always resolves with whatever it found (see + // url-discovery.service.ts). No outer reject-race here — that used to throw + // away every URL already discovered the moment the deadline passed, turning + // a slow-but-successful crawl into a 500 with an empty list. + const discoveryResult = await this.discoveryService.discoverUrls(discoveryOptions); // Prepare response const response: MapResponse = { @@ -93,7 +88,9 @@ export class MapController { url, includeSubdomains, maxUrls, - timestamp: new Date().toISOString() + timestamp: new Date().toISOString(), + // Signal a deadline-truncated result so callers can widen timeoutMs. + partial: discoveryResult.partial ?? false } }; diff --git a/src/api/schemas/index.ts b/src/api/schemas/index.ts index 5adf7b2..fd67433 100644 --- a/src/api/schemas/index.ts +++ b/src/api/schemas/index.ts @@ -175,7 +175,7 @@ export const mapRequestSchema = z.object({ skipSitemaps: z.boolean().optional().default(false), sitemapsOnly: z.boolean().optional().default(false), useUrlIndex: z.boolean().optional().default(true), - timeoutMs: z.number().int().min(1000).max(300000).optional().default(30000), + timeoutMs: z.number().int().min(1000).max(300000).optional().default(60000), includePatterns: z.array(z.string()).optional(), excludePatterns: z.array(z.string()).optional(), }); diff --git a/src/services/discovery-deadline.spec.ts b/src/services/discovery-deadline.spec.ts new file mode 100644 index 0000000..94fa611 --- /dev/null +++ b/src/services/discovery-deadline.spec.ts @@ -0,0 +1,47 @@ +import { withDeadline, makeDeadline } from './discovery-deadline'; + +const slow = (v: T, ms: number) => new Promise((r) => setTimeout(() => r(v), ms)); +const slowReject = (ms: number) => new Promise((_r, rej) => setTimeout(() => rej(new Error('boom')), ms)); + +describe('withDeadline', () => { + it('returns the promise value when it settles in time', async () => { + await expect(withDeadline(slow('ok', 5), 200, 'fallback')).resolves.toBe('ok'); + }); + + it('returns the fallback when the promise is too slow', async () => { + await expect(withDeadline(slow('ok', 200), 20, 'fallback')).resolves.toBe('fallback'); + }); + + it('returns the fallback (never rejects) when the promise rejects', async () => { + await expect(withDeadline(slowReject(5), 200, 'fallback')).resolves.toBe('fallback'); + }); + + it('returns the fallback immediately when ms <= 0', async () => { + await expect(withDeadline(slow('ok', 5), 0, 'fallback')).resolves.toBe('fallback'); + await expect(withDeadline(slow('ok', 5), -100, 'fallback')).resolves.toBe('fallback'); + }); + + it('does not lose the value if it resolves at nearly the same time (first settle wins)', async () => { + // Whatever wins the race, the result is one of the two valid outcomes and it never throws. + const r = await withDeadline(slow('ok', 20), 20, 'fallback'); + expect(['ok', 'fallback']).toContain(r); + }); +}); + +describe('makeDeadline', () => { + it('reports remaining budget and expiry against an injected clock', () => { + let t = 1000; + const d = makeDeadline(500, () => t); // ends at 1500 + expect(d.remaining()).toBe(500); + expect(d.expired()).toBe(false); + t = 1300; + expect(d.remaining()).toBe(200); + expect(d.expired()).toBe(false); + t = 1500; + expect(d.remaining()).toBe(0); + expect(d.expired()).toBe(true); + t = 1800; + expect(d.remaining()).toBe(0); // clamped, never negative + expect(d.expired()).toBe(true); + }); +}); diff --git a/src/services/discovery-deadline.ts b/src/services/discovery-deadline.ts new file mode 100644 index 0000000..f4b47a8 --- /dev/null +++ b/src/services/discovery-deadline.ts @@ -0,0 +1,54 @@ +/** + * Small deadline helpers for URL discovery. + * + * The map endpoint runs several discovery methods (sitemap, robots, common + * paths, subdomain sitemaps, browser crawl). Some are slow and unbounded — a + * site with a huge `docs.` sitemap can take far longer than the request's + * budget. These helpers let discovery treat `timeoutMs` as a SOFT deadline: + * return whatever has been found so far instead of throwing everything away. + * + * Kept as a pure, dependency-free module so the behaviour is unit-tested without + * standing up the browser pool / sitemap parser. + */ + +/** + * Resolve to `fallback` if `p` does not settle within `ms`. Never rejects — a + * rejection from `p` also resolves to `fallback`. The underlying work is not + * cancelled (there is no cross-cutting abort signal wired through discovery yet), + * it is just no longer awaited, so a slow method can't block the response. + */ +export function withDeadline(p: Promise, ms: number, fallback: T): Promise { + if (ms <= 0) return Promise.resolve(fallback); + return new Promise((resolve) => { + let settled = false; + const finish = (v: T) => { + if (settled) return; + settled = true; + clearTimeout(timer); + resolve(v); + }; + const timer = setTimeout(() => finish(fallback), ms); + // Don't let the timer keep the event loop alive on its own. + if (typeof timer.unref === 'function') timer.unref(); + p.then((v) => finish(v), () => finish(fallback)); + }); +} + +export interface Deadline { + /** Milliseconds left before the deadline (never negative). */ + remaining(): number; + /** True once the deadline has passed. */ + expired(): boolean; +} + +/** + * A monotonic-ish budget of `totalMs` from "now". `now` is injectable so the + * arithmetic is testable without real time. + */ +export function makeDeadline(totalMs: number, now: () => number = Date.now): Deadline { + const end = now() + totalMs; + return { + remaining: () => Math.max(0, end - now()), + expired: () => now() >= end, + }; +} diff --git a/src/services/url-discovery.service.ts b/src/services/url-discovery.service.ts index e55e382..630cfc1 100644 --- a/src/services/url-discovery.service.ts +++ b/src/services/url-discovery.service.ts @@ -2,6 +2,7 @@ import { SitemapParserService } from './sitemap-parser.service'; import { URLValidationUtils } from '../utils/url-validation.utils'; import { logger } from '../utils/logger'; import { assertPublicUrl, ssrfSafeRequestConfig } from '../utils/ssrf-guard'; +import { withDeadline, makeDeadline } from './discovery-deadline'; import { extractChildLinks } from '../scraper/crawl-links'; import { redisClient } from './redis.service'; import { PlaywrightService } from './playwright.service'; @@ -146,9 +147,11 @@ export class URLDiscoveryService { return cached; } - // Set up abort controller for timeout - const controller = new AbortController(); - const timeoutId = setTimeout(() => controller.abort(), timeoutMs); + // Treat timeoutMs as a SOFT deadline: each phase is bounded by the time left, + // and whatever has been discovered when the budget runs out is returned rather + // than thrown away. `timedOut` records that the result may be incomplete. + const deadline = makeDeadline(timeoutMs); + let timedOut = false; try { // Run discovery methods strategically: fast methods first, then supplement with crawling if needed @@ -177,25 +180,39 @@ export class URLDiscoveryService { sitemapPromises.push(this.runDocumentDiscovery(url, rateLimiting)); } - // Run fast discovery methods in parallel - const sitemapResults = await Promise.allSettled(sitemapPromises); - - // Process sitemap results - sitemapResults.forEach((result) => { - if (result.status === 'fulfilled') { - const methodResult = result.value; - allUrls.push(...methodResult.urls); - methodCounts[methodResult.method] += methodResult.urls.length; - } - }); + // Collect each method's URLs the moment it settles, then wait for the batch + // only up to the deadline. A method still running when the budget expires is + // dropped, but everything already found is kept — one hung sitemap can no + // longer erase the URLs the other methods returned. + const collect = (r: DiscoveryMethodResult) => { + allUrls.push(...r.urls); + methodCounts[r.method] += r.urls.length; + }; + const phase1 = Promise.allSettled( + sitemapPromises.map((p) => p.then(collect, () => { /* ignore per-method failure */ })) + ).then(() => true); + if ((await withDeadline(phase1, deadline.remaining(), false)) === false) { + timedOut = true; + } // Phase 1.5: Subdomain expansion — when includeSubdomains is set, actively - // discover sibling subdomains (docs., blog., …) and pull each sitemap. + // discover sibling subdomains (docs., blog., …) and pull each sitemap. This + // is the slow part (a big docs. site can have a huge sitemap), so it only + // runs while there's a meaningful budget left and is bounded by it. if (includeSubdomains && !skipSitemaps) { - const subResult = await this.runSubdomainDiscovery(url, maxUrls); - allUrls.push(...subResult.urls); - methodCounts.sitemap += subResult.urls.length; - logger.info(`Subdomain discovery added ${subResult.urls.length} URLs`, { url }); + if (deadline.remaining() > 2000) { + const subResult = await withDeadline( + this.runSubdomainDiscovery(url, maxUrls), + deadline.remaining(), + { urls: [], method: 'sitemap' as const, timeTaken: 0 } + ); + allUrls.push(...subResult.urls); + methodCounts.sitemap += subResult.urls.length; + logger.info(`Subdomain discovery added ${subResult.urls.length} URLs`, { url }); + } else { + logger.info('Skipping subdomain discovery: time budget exhausted', { url }); + } + if (deadline.expired()) timedOut = true; } // Phase 2: Browser crawling only if we haven't found enough URLs @@ -210,25 +227,36 @@ export class URLDiscoveryService { // would just add latency without finding meaningfully more. const sparseThreshold = Math.min(maxUrls, 30); if (!sitemapsOnly && currentUrlCount < sparseThreshold) { - logger.info('Running supplemental browser crawling', { - url, - currentUrls: currentUrlCount, - targetUrls: maxUrls - }); - - const crawlResult = await this.runBrowserDiscovery( - url, - Math.floor(maxUrls * 0.3), - rateLimiting, - { - ...crawlOptions, - maxCrawlDepth: Math.min(crawlOptions?.maxCrawlDepth || 2, 2), // Limit depth for speed - enableDeepCrawling: currentUrlCount < maxUrls * 0.5 // Only deep crawl if we really need more URLs - } - ); + // Browser crawling is the slowest method; skip it if the budget is spent. + if (deadline.remaining() > 3000) { + logger.info('Running supplemental browser crawling', { + url, + currentUrls: currentUrlCount, + targetUrls: maxUrls + }); - allUrls.push(...crawlResult.urls); - methodCounts.crawling += crawlResult.urls.length; + const crawlResult = await withDeadline( + this.runBrowserDiscovery( + url, + Math.floor(maxUrls * 0.3), + rateLimiting, + { + ...crawlOptions, + maxCrawlDepth: Math.min(crawlOptions?.maxCrawlDepth || 2, 2), // Limit depth for speed + enableDeepCrawling: currentUrlCount < maxUrls * 0.5 // Only deep crawl if we really need more URLs + } + ), + deadline.remaining(), + { urls: [], method: 'crawling' as const, timeTaken: 0 } + ); + + allUrls.push(...crawlResult.urls); + methodCounts.crawling += crawlResult.urls.length; + if (deadline.expired()) timedOut = true; + } else { + logger.info('Skipping browser crawling: time budget exhausted', { url, currentUrls: currentUrlCount }); + timedOut = true; + } } @@ -253,11 +281,16 @@ export class URLDiscoveryService { discoveryMethods: methodCounts, timeTaken, fromCache: false, - searchQuery + searchQuery, + // The soft deadline was hit: these results are usable but may be incomplete. + partial: timedOut || undefined }; - // Cache the result - await this.cacheResult(cacheKey, discoveryResult); + // Only cache complete results — a partial (timed-out) run shouldn't poison + // the cache with a truncated URL list for the full TTL. + if (!timedOut) { + await this.cacheResult(cacheKey, discoveryResult); + } logger.info('URL discovery completed', { url, @@ -275,8 +308,6 @@ export class URLDiscoveryService { timeTaken: Date.now() - startTime }); throw error; - } finally { - clearTimeout(timeoutId); } } diff --git a/src/types/discovery.ts b/src/types/discovery.ts index dd12150..c6a243a 100644 --- a/src/types/discovery.ts +++ b/src/types/discovery.ts @@ -48,6 +48,8 @@ export interface DiscoveryResult { timeTaken: number; fromCache: boolean; searchQuery?: string; + /** True when the soft deadline was hit and the URL list may be incomplete. */ + partial?: boolean; } export interface DiscoveryMethodResult { @@ -101,6 +103,8 @@ export interface MapResponse { includeSubdomains: boolean; maxUrls: number; timestamp: string; + /** True when discovery hit the soft deadline; widen timeoutMs for more. */ + partial?: boolean; }; error?: string; } diff --git a/swagger.yaml b/swagger.yaml index 9229c19..410bd3d 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -1428,7 +1428,7 @@ paths: type: integer minimum: 1000 maximum: 300000 - default: 30000 + default: 60000 includePatterns: type: array items: