aheckmann / node-ses

An Amazon SES api for nodejs with proper error handling.
http://aheckmann.github.com/node-ses
MIT License
201 stars 37 forks source link

Make work with eu-central-1 region #60

Closed epegzz closed 4 years ago

epegzz commented 4 years ago

This is really weird, but when using the eu-central-1 region the Connection header is not allowed to be present before signing, otherwise the request results in a SignatureDoesNotMatch error.

Thanks for @mhart for troubleshooting ๐Ÿ‘

markstos commented 4 years ago

Released in 3.0.0 today.

The release also bumps the aws4 dep and requests JSON from AWS internally instead of XML, so our direct XML deps are no longer needed.

epegzz commented 4 years ago

Awesome, thanks @markstos! ๐Ÿ‘

epegzz commented 4 years ago

@markstos Out of curiosity: Why did you decide to release a new major version? Are there any breaking API changes compared to 2.2.0?

epegzz commented 4 years ago

@markstos Did you maybe forgot to add a commit before tagging 3.0.0? Looks like 2.2.2 and 3.0.0 are the same commit.

markstos commented 4 years ago

2.2.2 should have been 3.0.0 due to the change from XML parsing to JSON parsing. See the History.md changelog notes for details.

https://github.com/aheckmann/node-ses/blob/master/History.md#300--2020-04-16

epegzz commented 4 years ago

The 3.0.0 tag points to this commit though: https://github.com/aheckmann/node-ses/commit/9dee0eced526f78e47c4195727dcd75d493006be

markstos commented 4 years ago

Yes.

On Fri, Apr 17, 2020 at 2:20 PM Daniel Schรคfer notifications@github.com wrote:

The 3.0.0 tag points to this commit though: 9dee0ec https://github.com/aheckmann/node-ses/commit/9dee0eced526f78e47c4195727dcd75d493006be

โ€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aheckmann/node-ses/pull/60#issuecomment-615393761, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGJZLSY4WIKPE4O3FBKCTRNCMWZANCNFSM4MIBZVZA .

-- Mark Stosberg Director of Systems and Security RideAmigos

epegzz commented 4 years ago

I really don't want to annoy you, but v3.0.0 still DOES use xml2js ๐Ÿ˜„

You tagged the same commit as 2.2.2 with 3.0.0

If in doubt, download v3.0.0 and check the source: In package.json it still says 2.2.1 and it lists xml2js as dependency

markstos commented 4 years ago

I just ran yarn add node-ses@3.0.0 and don't find mentions of xml in package.json or the lib directory.

Please provide steps to reproduce the problem.

   Mark

On Fri, Apr 17, 2020 at 5:03 PM Daniel Schรคfer notifications@github.com wrote:

I really don't want to annoy you, but v3.0.0 still DOES use xml2js ๐Ÿ˜„

You tagged the same commit as 2.2.2 with 3.0.0

If in doubt, download v3.0.0 and check the source :)

โ€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aheckmann/node-ses/pull/60#issuecomment-615462280, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGJZJNWX7VBC76RXQWWU3RNC7YZANCNFSM4MIBZVZA .

-- Mark Stosberg Director of Systems and Security RideAmigos

epegzz commented 4 years ago

Go to https://github.com/aheckmann/node-ses/releases

If you download the 3.0.0 package here then you can see that that version still includes the xml2js reference.

But you are right, on NPM it looks fine.

I had assumed that the commit tagged with 3.0.0 is also the commit that gets released on NPM as that version automatically via CI, but apparently you released the npm package manually (?) which would explain the discrepancy.

Anyhow, it's confusing but all good, NPM is what counts in the end ๐Ÿ‘