feat(type-safe-api): add s3 integration

[valebedu]

As discussed here #539, this is a proposition to add an s3 integration to type-safe-api

[cogwirrel]

Thanks so much for this!

I've added a few comments below :) Also please could you add some documentation here? Would be great to include some examples in the docs of how to read/write some simple json objects :)

This is a really great start! I can see this being a really useful addition to type-safe-api - thanks a bunch! 😄

[valebedu]

Sure, I totally forgot the documentation, I'll update it too

[github-actions]

This pull request is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

[github-actions]

Closing this pull request as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

[cogwirrel]

@valebedu just wondered if you're still working on this? Would be an awesome addition but no worries if you don't have the time! :)

[valebedu]

Hi @cogwirrel, yes it's just a bit difficult to find time 😅 I'll try to work on it this weekend

[cogwirrel]

Hi @cogwirrel, yes it's just a bit difficult to find time 😅 I'll try to work on it this weekend

All good, no rush! :) Will keep this PR alive 😄

[cogwirrel]

Thanks so much @valebedu! Just a few minor comments :) I think the main thing remaining if you don't mind is to make sure path parameters are mapped appropriately :)

[valebedu]

When viewing your example in docs I was thinking about the fixed status code with a DELETE and 200 is sometimes not what user wants.

So I added a prop to override it if needed

[cogwirrel]

So I added a prop to override it if needed

Thanks for this! 😄 It made me start thinking that users may even wish to customise other parts of the default/successful response, so I decided to generify your ErrorIntegrationResponse- Hope you don't mind! 😄 What do you think?

I was also playing around with a test project using your branch - it's so awesome! Love how path parameters just work and I can change the path/method easily :)

One slightly less intuitive thing I found was the default catchAll behaviour returning S3's xml errors in the response body, as the default model we vend for Type Safe API includes 400/403/404/500 error responses as a simple json object with a message property:

aws-pdk/packages/type-safe-api/src/project/model/smithy/smithy-definition.ts

Lines 169 to 176 in 09335a2

	/// An internal failure at the fault of the server
	@error("server")
	@httpError(500)
	structure InternalFailureError {
	/// Message with details about the error
	@required
	message: ErrorMessage
	}

Some kind of automatic coercion of the s3 xml error into the corresponding error response in the model would be the ideal way to handle it but I'd have thought that would be super complicated to build! As a compromise, I decided to make the default error behaviour IntegrationResponseSet.s3JsonErrorMessage() which extracts the S3 error message and returns it in a way which conforms to the default vended error models :)

[cogwirrel]

Awesome work @valebedu! Thanks so much for your contribution!

Given I've added a little bit to this I might get a quick second pair of eyes on this from @agdimech :)

[valebedu]

Awesome! Thank you so much :)

Yes, once you try S3 you find it really useful, I use it mostly in combinations with cron lambdas that run each several hours to get a specific result from a DB, writing it to a bucket an serving it through API Gateway, this is much cheaper and offers less pressure over DB for a result that doesn't change often.

And yes handling all S3 errors could be quite difficult to do. Maybe if this integration became popular with developers we will improve it :)

[agdimech]

This is looking really good, nice team work :)

[agdimech]

Looks good!!