fix(typescript): handle baseUrlId for single-URL WebSocket environments#13564
fix(typescript): handle baseUrlId for single-URL WebSocket environments#13564Swimburger merged 6 commits intomainfrom
Conversation
… channels When an AsyncAPI spec defines a named server (e.g. 'production'), the Fern IR converts it to a baseUrlId on the WebSocket channel. Previously, GeneratedSingleUrlEnvironmentsImpl.getReferenceToEnvironmentUrl() threw an error when baseUrlId was defined, even though for single-URL environments the baseUrlId is irrelevant since there's only one URL. This fix removes the error and simply returns the environment value as-is, allowing WebSocket client generation to work with single-URL environments that have a baseUrlId. Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…ironments Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
Analysis: Safety of silently ignoring
|
| Implementation | baseUrlId behavior |
Rationale |
|---|---|---|
EmptyGeneratedEnvironmentsImpl |
Ignores baseUrlId (not even in signature) |
No environments defined — just returns the input expression |
GeneratedSingleUrlEnvironmentsImpl |
Now ignores baseUrlId (this PR) |
Only one URL exists — baseUrlId can't select between URLs |
GeneratedMultipleUrlsEnvironmentsImpl |
Requires baseUrlId — uses it to select env.api vs env.auth etc. |
Multiple URLs — baseUrlId is needed to pick the right one |
The two callers both pass channel.baseUrl ?? undefined (WebSocket) or endpoint.baseUrl ?? undefined (HTTP):
GeneratedDefaultWebsocketImplementation.getEnvironment()(line 611-614) — WebSocket channels from AsyncAPI specs with named servers will have abaseUrlIdGeneratedSdkClientClassImpl.getEnvironment()(line 955-958) — HTTP endpoints may have abaseUrlIdwhen using multiple base URLs
For single-URL environments, the baseUrlId is purely informational — it can't change the outcome since there's only one URL. The generated code is return referenceToEnvironmentValue regardless.
Warning vs. silent ignore
A warning could be useful during development but would be noisy in production since this is a completely valid and common scenario (any AsyncAPI spec with a named server triggers this). I'd lean toward keeping it silent, but I could add a debug-level log if preferred.
Seed test coverage
The existing websocket seed fixture (test-definitions/fern/apis/websocket/) doesn't define environments. To cover this specific scenario, we'd need a new seed fixture (or extend the existing one) that:
- Defines a single-URL environment (e.g.,
environments: Production: https://api.example.com) - Has a WebSocket channel with a
baseUrlset (simulating what the AsyncAPI converter does for named servers) - Generates with
generateWebSocketClients: true
This would verify the generated code compiles correctly when baseUrlId is present on a WebSocket channel in a single-URL environment.
The unit test added in this PR ("getReferenceToEnvironmentUrl ignores baseUrlId for single URL") covers the core logic. A seed test would additionally verify the end-to-end generation pipeline. Happy to add one if desired.
| // For single URL environments, baseUrlId is irrelevant since there's only one URL. | ||
| // This can happen when an AsyncAPI spec defines a named server (e.g. "production") | ||
| // which gets converted to a baseUrlId on the WebSocket channel. |
There was a problem hiding this comment.
🔴 The test at EnvironmentsGenerator.test.ts:118 ("getReferenceToEnvironmentUrl throws when baseUrlId is provided") asserts .toThrow() when baseUrlId is non-null, but this PR removes that throw. This test will fail in CI and needs to be updated to expect the method to return the input expression instead of throwing.
Extended reasoning...
What the bug is
The test file generators/typescript/sdk/environments-generator/src/__test__/EnvironmentsGenerator.test.ts at lines 118-131 contains a test case titled "getReferenceToEnvironmentUrl throws when baseUrlId is provided". This test creates a GeneratedSingleUrlEnvironmentsImpl instance and asserts that calling getReferenceToEnvironmentUrl with a non-null baseUrlId throws an error.
The specific code path
The PR removes the if (baseUrlId != null) { throw new Error(...) } block from GeneratedSingleUrlEnvironmentsImpl.getReferenceToEnvironmentUrl() (lines 99-101 in the original code). The method now unconditionally returns referenceToEnvironmentValue. However, the test file is not included in the PR changes — only GeneratedSingleUrlEnvironmentsImpl.ts is modified.
Step-by-step proof
- The test at line 118 creates a
GeneratedSingleUrlEnvironmentsImplwith single-URL environments. - It creates an input expression
ts.factory.createIdentifier("env"). - It calls
expect(() => impl.getReferenceToEnvironmentUrl({ referenceToEnvironmentValue: inputExpr, baseUrlId: "some-base-url" })).toThrow(). - Before this PR, the method would throw
Error("Cannot get reference to single environment URL because baseUrlId is defined..."), so the test passed. - After this PR, the method returns
referenceToEnvironmentValuewithout throwing, so.toThrow()will fail — the function completes successfully instead of throwing.
Impact
This will cause a test failure in CI. The PR description itself has the "Unit tests added/updated" checkbox unchecked, which corroborates that the test was not updated.
How to fix
Update the test at lines 118-131 to match the new behavior. Instead of asserting .toThrow(), it should assert that the method returns the input expression (similar to the test at lines 104-116 which tests with baseUrlId: undefined). The updated test should look like:
it("getReferenceToEnvironmentUrl returns input when baseUrlId is provided", () => {
// ... same setup ...
const result = impl.getReferenceToEnvironmentUrl({
referenceToEnvironmentValue: inputExpr,
baseUrlId: "some-base-url"
});
expect(getTextOfTsNode(result)).toBe("env");
});When an AsyncAPI spec has a single named server and no HTTP servers, buildEnvironments creates a single-URL environment. Setting urlId on the channel in this case produces a baseUrlId in the Fern IR that is meaningless for single-URL environments and can cause generator issues. This change skips setting urlId when there are no HTTP servers and at most one WebSocket server, preventing the unnecessary baseUrlId from being generated in the first place. Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
Updated 3 snapshot files to reflect that single-WebSocket-server-only specs no longer set url: production on channels. Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
Verification of all three review checklist items1.
|
| Scenario | servers.length |
websocketServers.length |
Condition | urlId suppressed? |
|---|---|---|---|---|
| Single WS server, no HTTP (affected) | 0 | 1 | true |
✅ Yes — correct, single-URL env |
| No servers at all | 0 | 0 | true |
✅ Yes — correct, no server to reference |
| Multiple WS servers, no HTTP | 0 | 2+ | false |
❌ No — urlId preserved for multi-URL env |
| Single WS + HTTP servers | 1+ | 1 | false |
❌ No — urlId preserved for multi-URL env |
| Multiple WS + HTTP servers | 1+ | 2+ | false |
❌ No — urlId preserved for multi-URL env |
Confirmed by existing test fixtures:
env-single-no-endpoint-async,env-multiple-no-endpoint-async,env-single-with-endpoint-async,env-multiple-with-endpoint-async— all have 2 WS servers (websocket+websocket2), sowebsocketServers.length > 1→ condition isfalse→ their snapshots still have"url": "websocket"/"url": "websocket2"✅env-exhaustive-multi-multi— has 2 WS servers + HTTP servers → condition isfalse→ snapshot preserves allurlvalues ✅
Why the condition aligns with buildEnvironments: Lines 471-486 of buildEnvironments.ts show the single-URL environment creation path — it's entered when context.ir.servers.length === 0 and iterates over websocketServersWithName. When there's only one WS server, this creates a single-URL environment. The hasSingleWebsocketServerOnly condition in buildChannel.ts mirrors exactly this logic.
2. url: undefined in updated snapshots is correct
All 3 affected fixtures define exactly one production WS server with no HTTP servers:
asyncapi-v2-x-fern-parameter-name:servers.production: { url: wss://api.example.com, protocol: wss }— single WS, no HTTP, no OpenAPI companionasyncapi-v3-message-refs:servers.production: { host: api.example.com, protocol: wss }— single WS, no HTTP, no OpenAPI companionasyncapi-v3-x-fern-sdk-method-name:servers.production: { host: api.example.com, protocol: wss }— single WS, no HTTP, no OpenAPI companion
For all three, buildEnvironments creates a single-URL environment. Setting url: "production" (i.e. baseUrlId) on the channel would reference a URL ID that doesn't exist in the single-URL environment, which is what caused the generator crash. url: undefined is the correct output.
3. Generator-side fix is still valuable as defense-in-depth
Yes, it remains valuable. Even with the converter fix preventing this specific path, the generator fix provides a safety net because:
-
Other IR sources: The IR isn't only produced by the AsyncAPI converter. Custom Fern definitions, other importers, or future changes could produce a
baseUrlIdon a WebSocket channel with a single-URL environment. -
Semantic correctness: For single-URL environments,
baseUrlIdis genuinely meaningless — there's only one URL to resolve to regardless of whichbaseUrlIdis requested. TheEmptyGeneratedEnvironmentsImplalready follows this pattern (silently ignoresbaseUrlId). The fix makesSingleUrlconsistent. -
Contrast with
MultipleUrls:GeneratedMultipleUrlsEnvironmentsImpl.getReferenceToEnvironmentUrl()correctly throws whenbaseUrlIdisundefinedor not found, because for multi-URL environments it's essential to select the right URL. The single-URL impl doesn't need this — there's no selection to make. -
Unit test coverage (lines 118-130 of
EnvironmentsGenerator.test.ts): The test explicitly passesbaseUrlId: "some-base-url"and asserts the input expression is returned unchanged, confirming the graceful ignore behavior.
Description
Refs: Devin Session
Requested by: @Swimburger
Fixes a crash in the TypeScript SDK generator when generating WebSocket clients for AsyncAPI specs that define a named server (e.g.
servers.production) with a single-URL environment. Applies a two-pronged fix: the converter no longer emits an unnecessarybaseUrlId, and the generator gracefully handles one if it appears.Changes Made
Converter (input-side fix)
buildChannel.ts, skip settingurlIdon the channel when the spec has no HTTP servers and at most one WebSocket server. In this casebuildEnvironmentscreates a single-URL environment, so abaseUrlIdon the channel would be meaningless.Generator (defense-in-depth)
GeneratedSingleUrlEnvironmentsImpl.getReferenceToEnvironmentUrl()whenbaseUrlIdis non-nullbaseUrlIdis now silently ignored since there's only one URL to resolve toRoot cause: When an AsyncAPI spec defines a named server, the Fern IR converter assigns a
baseUrlIdto the WebSocket channel. The WebSocket client generator then passes thisbaseUrlIdtogetReferenceToEnvironmentUrl(). For multi-URL environments,baseUrlIdselects the correct URL. For single-URL environments, it's irrelevant — but the old code threw an error instead of proceeding.Testing
asyncapi-v2-x-fern-parameter-name,asyncapi-v3-message-refs,asyncapi-v3-x-fern-sdk-method-name) now reflecturl: undefinedinstead ofurl: "production"for single-WS-server specsshouldGenerateWebsocketClients: truefrom an AsyncAPI spec with a namedproductionserver. Generation succeeds and the produced WebSocket client connects correctly.Review Checklist
hasSingleWebsocketServerOnlycondition inbuildChannel.ts— it checkscontext.ir.servers.length === 0 && context.ir.websocketServers.length <= 1. Confirm this doesn't inadvertently suppressurlIdfor specs that have multiple WS servers or mixed HTTP+WS servers.url: undefinedin the updated snapshots is correct — these fixtures all define a singleproductionWS server with no HTTP servers, so removing theurlIdis the expected behavior.