feat(api): add custom endpoint support to API #14086

svidgen · 2024-12-17T21:04:20Z

Description of changes

Adds optional endpoint and apiKey properties to generateClient() options, allowing customers to use Amplify to connect to any accessible GraphQL API from within an app.

Also adds support for subscribing to multiple endpoints simultaneously.

When an alternative endpoint is provided:

authMode must be explicitly set. Omitting it is both a type error and runtime error.

authMode: "apiKey" requires that apiKey also be explicitly set; whether this mode is set in generateClient() or .graphql() downstream. Omitting it is both a type error and runtime error.

Issue #, if available

Description of how you validated changes

Tests added.
Manual testing
e2e's currently being authored

Checklist

PR description included
yarn test passes
Unit Tests are changed or added
Relevant documentation is changed or added (and PR referenced)

Checklist for repo maintainers

Verify E2E tests for existing workflows are working as expected or add E2E tests for newly added workflows
New source file paths included in this PR have been added to CODEOWNERS, if appropriate

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…endpoint

of testing depends on creating appsync connections to nowhere

stocaaro · 2024-12-18T22:40:30Z

packages/api-graphql/src/internals/InternalGraphQLAPI.ts

+		const memoKey = appSyncGraphqlEndpoint ?? 'none';
+		const realtimeProvider =
+			this.appSyncRealTime.get(memoKey) ?? new AWSAppSyncRealTimeProvider();
+		this.appSyncRealTime.set(memoKey, realtimeProvider);


This overwrites the memoKey, even when we just retrieved the provider from the set doesn't it? It's probably not doing harm, but seems like an unnecessary operation.

Yeah. I weighed it against a conditional check to do the same. This seemed simpler to read. But, I don't mind changing it if you disagree.

The conditional check seems like fewer moving parts or things to go wrong, but I also don't think I can come up with an example of when this would fail us, so this is a nit at best.

stocaaro · 2024-12-18T22:48:10Z

packages/api-graphql/src/internals/generateClient.ts

+	if (!client[__endpoint]) {
+		if (isApiGraphQLConfig(apiGraphqlConfig)) {
+			addSchemaToClient(client, apiGraphqlConfig, getInternals);
+		} else {


The old code would always either run addSchemaToClient or generateModelsPropertyOnAmplifyConfigure where the new code could run neither when there is an endpoint in the params and we don't raise the auth errors above, right?

Is it a problem to return a client without running one of these two functions?

Thinking over the change, probably not since we don't have a schema or models to bind in these additional endpoint use-cases. Interesting.

It's very deliberately skipping both to avoid attaching .models etc. and potentially causing confusion there — if someone were logging out the client and poking around for example, they'd see methods from their MIS that have nothing to do with the endpoint.

stocaaro · 2024-12-18T22:49:18Z

packages/api-graphql/src/internals/server/generateClientWithAmplifyInstance.ts

+	if (client[__endpoint]) {
+		if (!client[__authMode]) {
+			throw new Error(


This error logic is repeated from above, is other similar code shared in some way?

It's probably more LOC to do so to accommodate the slightly different error messaging. Is it worth it to you?

nit. Probably not worth it.

packages/api-graphql/src/internals/v6.ts

packages/api-graphql/src/server/generateClient.ts

stocaaro · 2024-12-18T22:56:30Z

packages/api-graphql/src/types/index.ts

+		? {
+				query: TYPED_GQL_STRING | DocumentNode;
+				variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+				authMode?: GraphQLAuthMode;
+				apiKey?: string;
+				authToken?: string;
+				/**
+				 * @deprecated This property should not be used
+				 */
+				userAgentSuffix?: string;
+			}
+		:
+				| {
+						query: TYPED_GQL_STRING | DocumentNode;
+						variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+						authMode?: never;
+						apiKey?: never;
+						authToken?: string;
+						/**
+						 * @deprecated This property should not be used
+						 */
+						userAgentSuffix?: string;
+				  }
+				| {
+						query: TYPED_GQL_STRING | DocumentNode;
+						variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+						authMode: 'apiKey';
+						apiKey: string;
+						authToken?: string;
+						/**
+						 * @deprecated This property should not be used
+						 */
+						userAgentSuffix?: string;
+				  }
+				| {
+						query: TYPED_GQL_STRING | DocumentNode;
+						variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+						authMode: Exclude<GraphQLAuthMode, 'apiKey'>;
+						apiKey?: never;
+						authToken?: string;
+						/**
+						 * @deprecated This property should not be used
+						 */
+						userAgentSuffix?: string;
+				  }


Would this be better expressed as a generic where we pass in override types to we can repeat it with different overrides to create this union? This seems like an easy point to accidentally inject errors trying to make simple modifications all spelled out like this.

FWIW, I did go down the generic route and found that TS was too confused to offer suggestions while typing (pressing ctrl+space in particular). I'm not sure if I made a mistake, but I then had to go on a small crusade to simplify the types down into the "dumbest" possible versions of themselves and it seemed to contribute to resolving the issue.

Hmm. Don't think we have the prettify wrapper here, but I think that might improve the CX on a more concise solution. Not sure where I land on this, this seems pretty long form.

stocaaro · 2024-12-18T23:05:53Z

packages/api-graphql/src/internals/InternalGraphQLAPI.ts

+		// attempting to connect to nowhere. So, I'm treating as the opposite of
+		// a Chesterton's fence for now. (A fence I shouldn't build, because I don't
+		// know why somethings depends on its absence!)
+		const memoKey = appSyncGraphqlEndpoint ?? 'none';


Is this memoKey sufficient? Wondering if a websocket might ever be accessed with different authtypes to achieve different permissions where both would have the same endpoint (or something similar)

I think in all case, overriding a graphql calls authtype works, but maybe a client with authtype A will be recreated with authtype B and the dev will be surprised that they get the original instance with type A...

Any other config that might be needed in the memoKey to avoid surprises?

I think it should be OK. We have normally used a single socket for all connections regardless of the authmode needed by the subscription itself. In chatting with Ivan about how to do this, I think he leaned towards keeping connections pooled by client — in most cases, I think that's slightly better for customers. Fewer local resources consumed at the very least, etc.. It's marginal. I wouldn't die on that hill.

If it causes issues, I think this behavior is pretty OK for us to change later.

…client/main

stocaaro · 2024-12-19T22:22:59Z

packages/api-graphql/src/internals/InternalGraphQLAPI.ts

+		const memoKey = appSyncGraphqlEndpoint ?? 'none';
+		const realtimeProvider =
+			this.appSyncRealTime.get(memoKey) ?? new AWSAppSyncRealTimeProvider();
+		this.appSyncRealTime.set(memoKey, realtimeProvider);


The conditional check seems like fewer moving parts or things to go wrong, but I also don't think I can come up with an example of when this would fail us, so this is a nit at best.

stocaaro · 2024-12-19T22:23:46Z

packages/api-graphql/src/internals/server/generateClientWithAmplifyInstance.ts

+	if (client[__endpoint]) {
+		if (!client[__authMode]) {
+			throw new Error(


nit. Probably not worth it.

stocaaro · 2024-12-19T22:25:50Z

packages/api-graphql/src/types/index.ts

+		? {
+				query: TYPED_GQL_STRING | DocumentNode;
+				variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+				authMode?: GraphQLAuthMode;
+				apiKey?: string;
+				authToken?: string;
+				/**
+				 * @deprecated This property should not be used
+				 */
+				userAgentSuffix?: string;
+			}
+		:
+				| {
+						query: TYPED_GQL_STRING | DocumentNode;
+						variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+						authMode?: never;
+						apiKey?: never;
+						authToken?: string;
+						/**
+						 * @deprecated This property should not be used
+						 */
+						userAgentSuffix?: string;
+				  }
+				| {
+						query: TYPED_GQL_STRING | DocumentNode;
+						variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+						authMode: 'apiKey';
+						apiKey: string;
+						authToken?: string;
+						/**
+						 * @deprecated This property should not be used
+						 */
+						userAgentSuffix?: string;
+				  }
+				| {
+						query: TYPED_GQL_STRING | DocumentNode;
+						variables?: GraphQLVariablesV6<FALLBACK_TYPES, TYPED_GQL_STRING>;
+						authMode: Exclude<GraphQLAuthMode, 'apiKey'>;
+						apiKey?: never;
+						authToken?: string;
+						/**
+						 * @deprecated This property should not be used
+						 */
+						userAgentSuffix?: string;
+				  }


Hmm. Don't think we have the prettify wrapper here, but I think that might improve the CX on a more concise solution. Not sure where I land on this, this seems pretty long form.

packages/adapter-nextjs/src/api/generateServerClient.ts

packages/api/__tests__/SRR.test.ts

Jon Wire added 8 commits December 17, 2024 15:03

multi-endpoint base implementation

3274385

preid release

ce857c9

fix build

4f8e7a9

simplified and improved conditional types for custom endpoint

93b5136

fixed missing ctrl+space auto-suggest for generateClient

2b2c30f

missing adds from previous commit

c65e2b9

revert superfluous test changes to api-graphql

ba4c61d

bump api package size limit

7f6a612

svidgen changed the title ~~multi-endpoint base implementation~~ feat(api): add custom endpoint support to API Dec 18, 2024

Jon Wire added 3 commits December 18, 2024 11:28

updated graphql API singleton to use separate realtime providers per …

e61d28d

…endpoint

removed the newly added exception for missing endpoint for now; lots

27123f5

of testing depends on creating appsync connections to nowhere

revert GHA preid change for tagged releasing

332580f

svidgen marked this pull request as ready for review December 18, 2024 18:08

svidgen requested review from sktimalsina, cshfang, HuiSF, pranavosu and a team as code owners December 18, 2024 18:08

blakedunson approved these changes Dec 18, 2024

View reviewed changes

stocaaro reviewed Dec 18, 2024

View reviewed changes

Jon Wire added 2 commits December 19, 2024 14:11

refined some internal types, removing corresponding as any casts

a6c912c

Merge remote-tracking branch 'upstream/main' into feat/graphql-multi-…

9614453

…client/main

iartemiev previously approved these changes Dec 19, 2024

View reviewed changes

blakedunson approved these changes Dec 19, 2024

View reviewed changes

stocaaro previously approved these changes Dec 19, 2024

View reviewed changes

svidgen added 2 commits December 20, 2024 09:01

Merge branch 'main' into feat/graphql-multi-client/main

ff64e6b

Merge branch 'main' into feat/graphql-multi-client/main

9302b10

svidgen dismissed stocaaro’s stale review via 9302b10 December 20, 2024 16:03

svidgen dismissed iartemiev’s stale review via 9302b10 December 20, 2024 16:03

iartemiev previously approved these changes Dec 20, 2024

View reviewed changes

Merge branch 'main' into feat/graphql-multi-client/main

3549f4a

HuiSF requested changes Dec 23, 2024

View reviewed changes

packages/adapter-nextjs/src/api/generateServerClient.ts Outdated Show resolved Hide resolved

cshfang reviewed Dec 23, 2024

View reviewed changes

packages/api/__tests__/SRR.test.ts Outdated Show resolved Hide resolved

PR feedback

a89d135

svidgen dismissed iartemiev’s stale review via a89d135 December 23, 2024 16:38

iartemiev approved these changes Dec 23, 2024

View reviewed changes

svidgen requested review from HuiSF and cshfang December 23, 2024 16:52

HuiSF approved these changes Dec 23, 2024

View reviewed changes

iartemiev merged commit ca2e4b8 into main Dec 23, 2024
30 checks passed

iartemiev deleted the feat/graphql-multi-client/main branch December 23, 2024 17:24

iartemiev mentioned this pull request Jan 6, 2025

feat(data): add multi-endpoint instructions aws-amplify/docs#8186

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): add custom endpoint support to API #14086

feat(api): add custom endpoint support to API #14086

svidgen commented Dec 17, 2024 •

edited

Loading

stocaaro Dec 18, 2024

svidgen Dec 18, 2024

stocaaro Dec 19, 2024

stocaaro Dec 18, 2024

svidgen Dec 18, 2024

stocaaro Dec 18, 2024

svidgen Dec 18, 2024

stocaaro Dec 19, 2024

stocaaro Dec 18, 2024

svidgen Dec 18, 2024

stocaaro Dec 19, 2024

stocaaro Dec 18, 2024

svidgen Dec 18, 2024 •

edited

Loading

stocaaro Dec 19, 2024

stocaaro Dec 19, 2024

stocaaro Dec 19, 2024

feat(api): add custom endpoint support to API #14086

feat(api): add custom endpoint support to API #14086

Conversation

svidgen commented Dec 17, 2024 • edited Loading

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

Checklist for repo maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svidgen Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svidgen commented Dec 17, 2024 •

edited

Loading

svidgen Dec 18, 2024 •

edited

Loading