Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3.192.0: breaking change - client.config.endpoint() is no longer automatically set #4122

Closed
3 tasks done
thetutlage opened this issue Oct 31, 2022 · 23 comments
Closed
3 tasks done
Assignees
Labels
bug This issue is a bug. guidance General information and guidance, answers to FAQs, or recommended best practices/resources. p0 This issue is the highest priority workaround-available This issue has a work around available.

Comments

@thetutlage
Copy link

Checkboxes for prior research

Describe the bug

The S3Client imported from @aws-sdk/client-s3 package has a breaking change in version 3.192.0. Following is the code to reproduce the issue

const { S3Client } = require('@aws-sdk/client-s3')

const client = new S3Client(config)

client.config.endpoint().catch(console.log)

The endpoint method exists in version 3.190.0, but has been removed from the runtime code in 3.192.0.

SDK version number

@aws-sdk/client-s3@3.192.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.11.0

Reproduction Steps

Installed the mentioned version of the client-s3 and execute the following code.

const { S3Client } = require('@aws-sdk/client-s3')

const client = new S3Client(config)

client.config.endpoint().catch(console.log)

Observed Behavior

Exception is raised

TypeError: client.config.endpoint is not a function

Expected Behavior

Should work

Possible Solution

No response

Additional Information/Context

No response

@thetutlage thetutlage added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 31, 2022
@RanVaknin RanVaknin self-assigned this Oct 31, 2022
@RanVaknin
Copy link
Contributor

@thetutlage ,

Please update your '@aws-sdk/client-s3' version. We are now at "^3.199.0" and you are several patches behind.

My code:

import { S3Client } from '@aws-sdk/client-s3';


const s3 = new S3Client({ 
    region: 'us-east-1',
    endpoint: "localhost://myendpoint.com"
});

const endpoint = await s3.config.endpoint()
console.log(endpoint)

Result:

{
  hostname: 'myendpoint.com',
  port: undefined,
  protocol: 'localhost:',
  path: '',
  query: undefined
}

Let me know if this helps.
Thanks,
Ran~

@RanVaknin RanVaknin added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 31, 2022
@thetutlage
Copy link
Author

Still the same error. One thing to note, I have not defined the endpoint property inside the config object.

const s3 = new S3Client({ 
    region: 'us-east-1'
});

const endpoint = await s3.config.endpoint()

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Nov 1, 2022
@RanVaknin
Copy link
Contributor

RanVaknin commented Nov 1, 2022

Hi @thetutlage ,

Thanks for the follow information.

I can confirm that this behavior changed, as running this in ^3.190.0 did not raise this error.
Encountering this error when endpoint is not provided in the client's config.

Repro code:

//node v18.0.0

import { S3Client } from '@aws-sdk/client-s3';

const s3 = new S3Client({region: "us-east-1"})
await s3.config.endpoint()

Error:

await s3.config.endpoint()
                ^

TypeError: s3.config.endpoint is not a function
    at 4122_endpoint_error/sample.js:5:17
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)
    at async loadESM (node:internal/process/esm_loader:85:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

Node.js v18.0.0

dependencies:

$ npm list
4122_endpoint_error
└── @aws-sdk/client-s3@3.200.0

$ npm list -all
<entire tree structure redacted>
@aws-sdk/util-endpoints@3.200.0
@aws-sdk/middleware-stack@3.200.0
(and all other middleware modules are 3.200.0)

Assigning to the dev team for further investigation.

Thanks,
Ran~

@RanVaknin RanVaknin added needs-review This issue/pr needs review from an internal developer. p1 This is a high priority issue labels Nov 1, 2022
@kuhe
Copy link
Contributor

kuhe commented Nov 1, 2022

All AWS SDKs have recently updated their endpoint implementations.

Here in the AWS SDK for JavaScript v3, the endpoint config field is now only for supplying an endpoint override. If you do not supply an override the normalized function cannot be called.

To resolve a service endpoint without calling an API operation, you can reference this example.

Both a client and a command are needed, because the endpoint may now vary depending on the parameters of both, and which command is being used.

const { S3Client, PutObjectCommand } = require("@aws-sdk/client-s3");
const { getEndpointFromInstructions } = require("@aws-sdk/middleware-endpoint");

(async () => {
  // 1. a service client
  const client = new S3Client({});

  // 2. a command
  const command = new PutObjectCommand({
    Key: "__key__",
    Bucket: "__bucket__",
  });

  const endpoint = await getEndpointFromInstructions(command.input, PutObjectCommand, client.config);
  console.log(endpoint);
})();

output:

{
  headers: {},
  properties: { authSchemes: [ [Object] ] },
  url: URL {
    href: 'https://s3.us-east-1.amazonaws.com/__bucket__',
    origin: 'https://s3.us-east-1.amazonaws.com',
    protocol: 'https:',
    username: '',
    password: '',
    host: 's3.us-east-1.amazonaws.com',
    hostname: 's3.us-east-1.amazonaws.com',
    port: '',
    pathname: '/__bucket__',
    search: '',
    searchParams: URLSearchParams {},
    hash: ''
  }
}

@kuhe kuhe added guidance General information and guidance, answers to FAQs, or recommended best practices/resources. workaround-available This issue has a work around available. and removed bug This issue is a bug. needs-review This issue/pr needs review from an internal developer. p1 This is a high priority issue labels Nov 1, 2022
@kuhe kuhe changed the title Breaking change in 3.192.0 v3.192.0: breaking change - client.config.endpoint() is no longer automatically set Nov 1, 2022
@thetutlage
Copy link
Author

@kuhe So what you are telling me is?

  • You have pushed a breaking change in between a minor version.
  • And did not even mention it in any release notes
  • And the TypeScript typings tells that the endpoint method always exists.

@bizob2828
Copy link

I'm with @thetutlage. I'm going to voice my frustrations with this project recently. It no longer follow semantic versioning. As an agent developer for New Relic, these updates have been very disruptive. We do not know how to handle this package going forward. We don't want to lock down only instrumenting to a specific version but there's been a rash of breakages that are forcing our hand.

@bizob2828
Copy link

I'm still unsure why the bug label has been removed here. At the very least the typescript typings need updated

@kuhe
Copy link
Contributor

kuhe commented Nov 8, 2022

These packages aren't semantically versioned.

  • update typings and jsdoc to indicate that endpoint will not resolve unless supplied.

@kuhe kuhe added bug This issue is a bug. p1 This is a high priority issue labels Nov 8, 2022
@kuhe kuhe self-assigned this Nov 8, 2022
@thetutlage
Copy link
Author

Is there any specific reason for removing this method in first place?

@thetutlage
Copy link
Author

To share more context. I was relying on this method to generate the URLs for an object inside the bucket. Is there an alternate API that can return the URL of an object, especially when the config does not have endpoint configured

@bizob2828
Copy link

These packages aren't semantically versioned.

  • update typings and jsdoc to indicate that endpoint will not resolve unless supplied.

@kuhe it would be super helpful if this was called out at the top of the README. It's very unusual for npm packages, especially from such a reputable company as AWS, to create open source packages and not adhere to semantic versioning.

@bizob2828
Copy link

To share more context. I was relying on this method to generate the URLs for an object inside the bucket. Is there an alternate API that can return the URL of an object, especially when the config does not have endpoint configured

@thetutlage I think @kuhe provided a solution here, did that work for you?

@thetutlage
Copy link
Author

@bizob2828 Nope, if I use the following code, then TypeScript complains for types mis-match. Their SDKs do not have any docs, its just auto generated methods signature, so I have no idea how to approach creating URLs in practice.

Even if I stumble upon some API, I cannot be sure that the API will not be removed a week from now.

const command = new GetObjectCommand({
    Key: "__key__",
    Bucket: "__bucket__",
  });

  const endpoint = await getEndpointFromInstructions(command.input, GetObjectCommand, client.config);

@RanVaknin RanVaknin added p0 This issue is the highest priority and removed p1 This is a high priority issue labels Nov 8, 2022
@kuhe
Copy link
Contributor

kuhe commented Nov 9, 2022

@thetutlage

Is there any specific reason for removing this method in first place?

Due to a recent change in AWS SDKs, AWS service endpoints are now resolved by an internal ruleset, found in client-*/endpoint/ruleset.ts. Example: https://github.com/kuhe/aws-sdk-js-v3/blob/1437c0f/clients/client-s3/src/endpoint/ruleset.ts

Because of the potential complexity of these rulesets, a service endpoint can no longer be resolved without additional pieces of information from the various Command and Config objects.

While you might still have a reasonably good chance of getting the correct endpoint by using {servicename}.{region}.amazonaws.com, there will be many cases for which this does not work. Therefore, the config.endpoint method can no longer be provided.

generate the URLs for an object inside the bucket

You should be able to use a generic S3 URL format like
https://s3.region-code.amazonaws.com/bucket-name/key-name or https://bucket-name.s3.region-code.amazonaws.com/key-name s3 docs. There are many more forms of S3 Endpoint as can be seen in the ruleset.ts file, but they are not necessarily valid as object URLs, so I'm not sure if you should be using an endpoint resolver to get an object URL.


@bizob2828
re: semver

called out at the top of the README

  • I plan to submit for team review a proposed update to the top level readme doc. It should describe the versioning system, and set expectations for the API stability level of our @aws-sdk/client-*s and other packages.

typings fix

I submitted a PR to attempt to stabilize the client config API from this type of disruption: #4156. This also contains the type fix to set endpoint in the resolved config as optional.


re: using internal API to resolve endpoint

The code I provided works with the latest versions of @aws-sdk/* packages to resolve an endpoint by ruleset for a given service and command. If you still need to resolve an endpoint, check your package.lock versions for consistency. This is not a public API of this AWS SDK JSv3, though you could "feature request" it with a new issue.


P.S. I apologize for the disruption these updates and bugs are causing to your applications. The above steps are intended to prevent further occurrences.

@rcoundon
Copy link

rcoundon commented Nov 9, 2022

Just to add my tuppence worth. We have never specified an endpoint but we do specify a region in the creation of our SQSClient (and others like S3, SSM etc). So this raises a number of questions:

  1. Is this now saying we must now specify an endpoint?
  2. If so, how do we know what we should specify?
  3. Is this the case for all v3 SDK Clients?

@kuhe
Copy link
Contributor

kuhe commented Nov 9, 2022

There are two similar but distinct object types associated with a client config.

One is the config input type, what you send into const client = new ServiceClient({ /* config input */ }) and the other is what it becomes afterward, called the "resolved config" at client.config. The resolved config fills in default values and does other normalization. One of these previously was the injection of an endpoint function.

The endpoint has always been optional in the input, and continues to be optional. You do not need to provide a custom endpoint. This thread is about the resolved type no longer containing an endpoint function if you did not provide an endpoint or endpoint function as input.

For example, in S3Client.ts in this repo, the two are called type S3ClientConfigType and type S3ClientResolvedConfigType respectively.

@rcoundon
Copy link

rcoundon commented Nov 9, 2022

I see, thanks for clarifying. My interest is related to this where we were only using the config input type, as you describe it. Others seemed to have had the same issue.

@ianwremmel
Copy link

How does one use getEndpointFromInstructions to feed into a command?

I used to be able to use the following code to delete a handled SQS message

  const endpoint = await sqs.config.endpoint()

  const queueUrl = url.format({
    ...endpoint,
    pathname: path.join(endpoint.path, accountId, queueName),
  });

  return await sqs.send(
    new DeleteMessageCommand({
      QueueUrl: queueUrl.toString(),
      ReceiptHandle: receiptHandle,
    })
  );

but the new equivalent is invalid.

  const fakeDeleteCommand = new DeleteMessageCommand({
    ReceiptHandle: receiptHandle,
  });

  const endpoint = await getEndpointFromInstructions(
    fakeDeleteCommand.input,
    DeleteMessageCommand,
    sqs.config
  );

  const queueUrl = url.format({
    ...endpoint,
    pathname: path.join(endpoint.path, accountId, queueName),
  });

  return await sqs.send(
    new DeleteMessageCommand({
      QueueUrl: queueUrl.toString(),
      ReceiptHandle: receiptHandle,
    })
  );
  • The instantiation of fakeDeleteCommand is invalid becaue there's no QueueUrl yet
  • getEndpointFromInstructions throws a type error because fakeDeleteCommand.input is a DeleteMessageCommandInput, not a Record<string, unknown>.
  • endpoint.path doesn't exist anymore

I guess maybe I can use GetQueueUrlCommand, but that looks like it's intended for cross-account stuff and would be overkill in this case.

@ianwremmel
Copy link

ok, turns out GetQueueUrlCommand won't work either (or, at least, I don't have anyway to determine the right input arguments. I'm just going to hold a v3.191.0 until y'all come up with a workable resolution.

@kuhe
Copy link
Contributor

kuhe commented Dec 2, 2022

The endpoint URL object from getEndpointFromInstructions is different from the string returned from endpoint(). It has a url field that is a string.

The public API to delete queues by name is to call getQueueUrl and then submit the deleteQueue command with the url from the first response.

@kuhe kuhe closed this as completed Dec 2, 2022
@ianwremmel
Copy link

That still doesn’t help with deleting a message. The DeleteMessage command needs a QueueUrl argument, but there no longer appears to be a way to figure out what the URL should be based on message alone.

@kuhe
Copy link
Contributor

kuhe commented Dec 5, 2022

in your example you use a queueName variable. That can be used with getQueueUrl.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. guidance General information and guidance, answers to FAQs, or recommended best practices/resources. p0 This issue is the highest priority workaround-available This issue has a work around available.
Projects
None yet
Development

No branches or pull requests

6 participants