fix: handle clientContextParam collisions with builtin config keys #1788

smilkuri · 2025-11-24T16:07:30Z

Issue #, if available:

Description of changes:

This fixes TypeScript client codegen conflict between known config keys and clientContextParams by adding the nested clientContextParams object to isolate conflicting parameters while maintaining backward compatibility and proper precedence.

--

packages/middleware-endpoint/src/adaptors/getEndpointFromInstructions.ts

packages/middleware-endpoint/src/adaptors/createConfigValueProvider.ts

...est/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2GeneratorTest.java

.changeset/stale-phones-behave.md

...rc/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java

private/my-local-model-schema/src/endpoint/EndpointParameters.ts

packages/middleware-endpoint/src/adaptors/createConfigValueProvider.ts

kuhe · 2025-12-02T16:38:09Z

packages/middleware-endpoint/src/adaptors/createConfigValueProvider.ts

+    if (isClientContextParam) {
+      // For client context parameters, check clientContextParams first
+      const clientContextParams = config.clientContextParams as Record<string, unknown> | undefined;
+      const nestedValue: unknown = clientContextParams?.[configKey] ?? clientContextParams?.[canonicalEndpointParamKey];


is there any case where the nested object does not use the canonical parameter key?

If not, there's no reason to check the configKey entry for the nested object.

I don't think so

packages/middleware-endpoint/src/adaptors/getEndpointFromInstructions.ts

kuhe · 2025-12-02T16:46:06Z

private/my-local-model/src/endpoint/EndpointParameters.ts

  options: T & ClientInputEndpointParameters
 ): T & ClientResolvedEndpointParameters => {
  return Object.assign(options, {
+    apiKey: options.apiKey ?? "default-api-key",


This is the main point of the fix, but this shouldn't appear here, since it will conflict with the same key potentially be added by the api key auth scheme.

This is generated because the parameter has a default value set defined in @endpointRuleSet. If a default value is not defined it doesn't generate this.

that does not matter - a conflicting key cannot be generated here

private/my-local-model/src/endpoint/ruleset.ts

...rc/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java

smithy-typescript-protocol-test-codegen/model/my-local-model/main.smithy

packages/eventstream-serde-universal/src/eventstream-cbor.integ.spec.ts

packages/middleware-endpoint/src/adaptors/createConfigValueProvider.spec.ts

packages/util-retry/src/retries.integ.spec.ts

kuhe · 2025-12-05T16:27:50Z

private/my-local-model-schema/src/auth/httpAuthSchemeProvider.ts

+  type Provider,
+  ApiKeyIdentity,
+  ApiKeyIdentityProvider,
+  HttpApiKeyAuthLocation,


these new imports look like types. Change the codegen part to use addTypeImport() instead, or if imported via symbol, ensure the symbol has the typeOnly property.

private/my-local-model-schema/src/endpoint/EndpointParameters.ts

kuhe · 2025-12-05T16:36:45Z

private/my-local-model-schema/src/endpoint/EndpointParameters.ts

 */
 export interface EndpointParameters extends __EndpointParameters {
-  endpoint?: string | undefined;
+  endpoint: string;


not saying it's wrong necessarily, but why did endpoint become required in the change?

kuhe · 2025-12-05T16:37:45Z

private/my-local-model/src/auth/httpAuthExtensionConfiguration.ts

@@ -1,5 +1,5 @@
 // smithy-typescript generated code
-import type { HttpAuthScheme } from "@smithy/types";
+import { type HttpAuthScheme, ApiKeyIdentity, ApiKeyIdentityProvider } from "@smithy/types";


change these to type imports

kuhe · 2025-12-05T16:38:02Z

private/my-local-model/src/auth/httpAuthSchemeProvider.ts

+  type Provider,
+  ApiKeyIdentity,
+  ApiKeyIdentityProvider,
+  HttpApiKeyAuthLocation,


use type imports

HttpApiKeyAuthLocation can't be changed to type only. It's accessing a runtime value HttpApiKeyAuthLocation.HEADER

smithy-typescript-protocol-test-codegen/model/my-local-model/main.smithy

kuhe · 2025-12-10T17:37:25Z

packages/eventstream-serde-universal/src/clientContextParams-precedence.integ.spec.ts

+      customParam: "user-custom-value",
+      clientContextParams: {
+        apiKey: "test-key",
+        customParam: "default-custom-value",


Suggested change

customParam: "default-custom-value",

customParam: "nested-custom-value",

this is not the default, it is given by the user at the client instantiation site

kuhe · 2025-12-10T17:38:20Z

packages/eventstream-serde-universal/src/clientContextParams-precedence.integ.spec.ts

+
+    // Verify the resolved config has the user's root-level value
+    const resolvedConfig = (clientWithNonConflicting as any).config;
+    expect(resolvedConfig.customParam).toBe("user-custom-value");


This should be "nested-custom-value". When a value exists in both places the nested value should be used.

kuhe · 2025-12-10T17:39:52Z

packages/eventstream-serde-universal/src/clientContextParams-precedence.integ.spec.ts

+    });
+
+    // Verify that both auth and endpoint contexts can coexist
+    const resolvedConfigConflicting = (clientWithConflicting as any).config;


why as any?

private/my-local-model-schema/src/XYZServiceClient.ts

private/my-local-model-schema/src/auth/httpAuthExtensionConfiguration.ts

...rc/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java

.../main/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinder.java

kuhe · 2025-12-10T17:55:07Z

smithy-typescript-protocol-test-codegen/model/my-local-model/main.smithy

+        enableFeature: { type: "Boolean", required: true, default: true, documentation: "Feature toggle with default" }
+        debugMode: { type: "Boolean", required: true, default: false, documentation: "Debug mode with default" }
+        nonConflictingParam: { type: "String", required: true, default: "non-conflict-default", documentation: "Non-conflicting with default" }
+        logger: { type: "String", required: true, default: "default-logger", documentation: "Conflicting logger with default" }


all endpoint param types need to be lowercase string boolean. Add a stringArray one too, like "additionalFeatures".

smilkuri force-pushed the resolve-conflict-params branch from 4a0b1ea to c0cc0c7 Compare November 26, 2025 16:57

smilkuri mentioned this pull request Nov 26, 2025

chore(codegen): generate corresponding clients for conflict params aws/aws-sdk-js-v3#7527

Closed

smilkuri force-pushed the resolve-conflict-params branch from f3ea1fe to 13fd0f0 Compare December 1, 2025 16:16

smilkuri marked this pull request as ready for review December 1, 2025 16:57

smilkuri requested a review from a team as a code owner December 1, 2025 16:57

smilkuri force-pushed the resolve-conflict-params branch 2 times, most recently from 9117008 to 1fc37f8 Compare December 1, 2025 17:01

smilkuri changed the title ~~fix: nest conflicting endpoint parameters~~ fix: handle clientContextParam collisions with builtin config keys Dec 1, 2025

kuhe requested changes Dec 1, 2025

View reviewed changes

smilkuri force-pushed the resolve-conflict-params branch from ef3cda8 to c5f0d86 Compare December 1, 2025 20:11

kuhe reviewed Dec 1, 2025

View reviewed changes

...rc/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java Outdated Show resolved Hide resolved

kuhe reviewed Dec 1, 2025

View reviewed changes

...rc/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java Outdated Show resolved Hide resolved

kuhe reviewed Dec 1, 2025

View reviewed changes

...rc/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java Outdated Show resolved Hide resolved

kuhe reviewed Dec 1, 2025

View reviewed changes

private/my-local-model-schema/src/endpoint/EndpointParameters.ts Show resolved Hide resolved

kuhe reviewed Dec 2, 2025

View reviewed changes

smilkuri force-pushed the resolve-conflict-params branch 3 times, most recently from 9f47441 to 1ed724d Compare December 4, 2025 18:09

smilkuri added 8 commits December 5, 2025 15:35

fix: nest conflicting endpoint parameters

13a1bca

fix: add fallback resolution

f2c3c14

chore: fix formatting and add changeset

1d493e5

chore: address comments and add diff for build generate-protocol-tests

d0657db

fix: add isClientContextParam flag to handle parameter types

6dc79b1

chore: refactor code and add custom ruleset

c81cf3d

chore: make code cleaner

c453d61

chore: fix failing integ tests

182326a

smilkuri force-pushed the resolve-conflict-params branch from 1ed724d to 182326a Compare December 5, 2025 15:55

smilkuri mentioned this pull request Dec 5, 2025

chore: codegen changes for handling conflicting params aws/aws-sdk-js-v3#7549

Open

kuhe reviewed Dec 5, 2025

View reviewed changes

chore: format protocol tests

30fc751

smilkuri force-pushed the resolve-conflict-params branch 2 times, most recently from 8965dee to 7016448 Compare December 10, 2025 15:54

fix: update clientContextParamDefaults to only have conflicting params

5401b57

smilkuri force-pushed the resolve-conflict-params branch from 7016448 to 5401b57 Compare December 10, 2025 15:57

smilkuri commented Dec 10, 2025

View reviewed changes

smithy-typescript-protocol-test-codegen/model/my-local-model/main.smithy Show resolved Hide resolved

kuhe reviewed Dec 10, 2025

View reviewed changes

	customParam: "default-custom-value",
	customParam: "nested-custom-value",

fix: handle clientContextParam collisions with builtin config keys #1788

Are you sure you want to change the base?

fix: handle clientContextParam collisions with builtin config keys #1788

Uh oh!

Conversation

smilkuri commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

smilkuri commented Nov 24, 2025 •

edited

Loading