fix: add timeout to registry URL validation to prevent CLI hang#2086
fix: add timeout to registry URL validation to prevent CLI hang#2086phusi319 wants to merge 2 commits intoasyncapi:masterfrom
Conversation
…capi#2027) Add AbortController with 10s timeout to registryValidation() fetch call. When --registry-url points to an unreachable host, the CLI now fails fast with a clear error instead of hanging indefinitely. Changes: - Use HEAD method instead of GET for lightweight validation - Add AbortController with 10s timeout - Differentiate timeout vs network vs auth errors in messages - Clear timer in finally block to prevent resource leaks - Add unit tests for URL parser and timeout behavior
phusi319
requested review from
AayushSaini101,
Amzani,
Shurtu-gal and
asyncapi-bot-eve
as code owners
🦋 Changeset detectedLatest commit: 6bba54c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
There was a problem hiding this comment.
Pull request overview
Adds a timeout to registry URL validation so the CLI fails fast instead of hanging indefinitely when a registry host is unreachable (Fixes #2027).
Changes:
- Add
AbortController+ 10s timeout and switch validation request toHEADinregistryValidation(). - Improve error messaging to distinguish timeout vs. general reachability failures.
- Add unit tests for
registryURLParser()andregistryValidation()behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/utils/generate/registry.ts |
Adds timeout/abort handling to prevent CLI hangs during registry validation. |
test/unit/utils/registry.test.ts |
Adds tests for URL validation and fast-failure behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('should fail fast for unreachable URLs instead of hanging', async () => { | ||
| const start = Date.now(); | ||
| try { | ||
| // 10.255.255.1 is a non-routable IP that will trigger the timeout | ||
| await registryValidation('http://10.255.255.1'); | ||
| expect.fail('Should have thrown'); | ||
| } catch (error: unknown) { | ||
| const elapsed = Date.now() - start; | ||
| expect(elapsed).to.be.lessThan(15_000); // Must resolve within 15s (10s timeout + margin) | ||
| expect(error).to.be.instanceOf(Error); | ||
| const msg = (error as Error).message; | ||
| expect(msg).to.satisfy( | ||
| (m: string) => m.includes('timed out') || m.includes('Unable to reach'), | ||
| `Expected timeout or unreachable error, got: ${msg}` | ||
| ); | ||
| } | ||
| }).timeout(20_000); |
There was a problem hiding this comment.
This unit test makes a real network call to http://10.255.255.1 and assumes it is unreachable. That IP is private (10/8) and can be routable in some CI/VPN/corporate environments, making the test flaky and potentially slow. Prefer stubbing global.fetch to a never-resolving promise that rejects on AbortSignal abort, so the timeout behavior is tested deterministically without external networking.
| } | ||
| }).timeout(20_000); | ||
|
|
||
| it('should throw auth error for 401 without credentials', async () => { |
There was a problem hiding this comment.
This test case is empty (no assertions and no skip), so it currently provides no coverage and can be misleading. Either implement it (e.g., stub fetch to return a Response with status 401) or mark it as skipped/todo so it doesn’t look like coverage exists when it doesn’t.
| it('should throw auth error for 401 without credentials', async () => { | |
| it.skip('should throw auth error for 401 without credentials', async () => { |
| if (response.status === 401 && !registryAuth && !registryToken) { | ||
| throw new Error('You Need to pass either registryAuth in username:password encoded in Base64 or need to pass registryToken'); | ||
| } | ||
| } catch { | ||
| throw new Error(`Can't fetch registryURL: ${registryUrl}`); | ||
| } catch (error: unknown) { | ||
| if (error instanceof Error && error.name === 'AbortError') { | ||
| throw new Error(`Registry URL validation timed out after ${REGISTRY_TIMEOUT_MS / 1000}s: ${registryUrl}`); | ||
| } | ||
| if (error instanceof Error && error.message.includes('registryAuth')) { | ||
| throw error; | ||
| } | ||
| throw new Error(`Unable to reach registry URL: ${registryUrl}`); |
There was a problem hiding this comment.
The auth error preservation currently relies on checking whether error.message contains the substring registryAuth. This is brittle (message changes will silently alter behavior) and could misclassify unrelated errors. Prefer restructuring so only the fetch call is inside the try/catch (rethrow non-fetch errors), or use a dedicated error type/constant for the 401-without-credentials case and check against that instead of parsing the message.
|
Fixes #2027
Problem
When
--registry-urlpoints to an unreachable host (e.g.,http://10.255.255.1),registryValidation()callsfetch()without a timeout. The CLI hangs indefinitely, stalling CI pipelines and leaving users without feedback.Root Cause
ypescript // registry.ts (before) const response = await fetch(registryUrl as string); // no timeout, no AbortControllerFix
Registry URL validation timed out after 10s: <url>Unable to reach registry URL: <url>finallyblock to prevent resource leaksChanges
src/utils/generate/registry.tstest/unit/utils/registry.test.tsTesting
registryURLParser: validates http/https, rejects invalid URLsregistryValidation: confirms unreachable URLs fail within 15s (10s timeout + margin) instead of hanging indefinitely