fix(cli): validate navigation timeout honors --timeout, hints on CDN scripts#1929
Draft
miguel-heygen wants to merge 1 commit into
Draft
fix(cli): validate navigation timeout honors --timeout, hints on CDN scripts#1929miguel-heygen wants to merge 1 commit into
miguel-heygen wants to merge 1 commit into
Conversation
…scripts `validate` navigated the page with a hardcoded 10s timeout that ignored the --timeout option. A composition that loads GSAP (or any library) from a CDN <script> in <head> blocks `domcontentloaded` until that script finishes downloading; on a slow network that exceeds 10s and validate fails with an opaque "Navigation timeout of 10000ms exceeded" — even though the full render (much larger budget) rides it out fine, and even though --timeout (the documented "wait longer for slow loads" knob) had no effect on navigation. The only recourse was to change the composition (vendor the script locally). Reported precisely, with the exact error and the observation that render's 60s budget masks it while validate's 10s trips. Fix: - resolveNavigationTimeoutMs(optTimeout) = max(10s floor, --timeout), so --timeout now also extends the navigation budget. Default behavior is unchanged: the default --timeout (3000) stays clamped to the 10s floor. - navigationTimeoutHint() replaces Puppeteer's opaque timeout error with an actionable message naming the likely cause (a blocking CDN <script>) and the two fixes (vendor locally / raise --timeout). Any non-timeout error is rethrown unchanged. - --timeout help text updated to note it also governs navigation. Both helpers are pure and exported; validateInBrowser wires them around the single page.goto. No behavior change for compositions that navigate within 10s. Test: resolveNavigationTimeoutMs (floor kept for unset/small/zero, raised past the floor) and navigationTimeoutHint (rewrites a nav-timeout error with CDN + --timeout guidance; returns null for other errors so the caller rethrows as-is). validate suite 14 tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
validatenavigated the page with a hardcoded 10s timeout (page.goto(..., { timeout: 10000 })) that ignored the--timeoutoption. A composition that loads GSAP (or any library) from a CDN<script>in<head>blocksdomcontentloadeduntil that script finishes downloading; on a slow network that exceeds 10s and validate fails with an opaque "Navigation timeout of 10000ms exceeded" — even though:--timeout(the documented "wait longer for slow loads" knob) had no effect on navigation, so the only recourse was to change the composition (vendor the script locally).Reported precisely, with the exact error and the observation that render's 60s budget masks it while validate's 10s trips.
Fix
resolveNavigationTimeoutMs(optTimeout)=max(10s floor, --timeout), so--timeoutnow also extends the navigation budget. Default behavior is unchanged: the default--timeout(3000) stays clamped to the 10s floor, and compositions that navigate within 10s are unaffected.navigationTimeoutHint()replaces Puppeteer's opaque timeout error with an actionable message naming the likely cause (a blocking CDN<script>) and the two fixes (vendor locally / raise--timeout). Any non-timeout error is rethrown unchanged.--timeouthelp text updated to note it also governs navigation.Both helpers are pure and exported;
validateInBrowserwires them around the singlepage.goto.Test plan
resolveNavigationTimeoutMs: floor kept for unset / default-3000 / zero; raised to--timeoutwhen it exceeds the floor.navigationTimeoutHint: rewrites a nav-timeout error with CDN +--timeoutguidance; returnsnullfor other errors so the caller rethrows them as-is.bunx vitest run packages/cli/src/commands/validate.test.ts— 14/14 passing (was 9).tsc --noEmitclean; fullbun run buildclean;oxlint/oxfmtclean.