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

Fix streaming APIs' Err method closing stream #2882

Merged
merged 3 commits into from Oct 21, 2019

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Oct 9, 2019

Fixes calling the Err method on SDK's Amazon Kinesis's SubscribeToShared and Amazon S3's SelectObjectContent response EventStream members closing the stream. This would cause unexpected read errors, or early termination of the streams. Only the Close method of the streaming members will close the streams.

Improved generated unit and integration tests for streaming API operations.

Related to #2769

@jasdel jasdel added the needs-review This issue or pull request needs review from a core team member. label Oct 9, 2019
if !ok {
t.Fatalf("expect stream not to be closed, but was")
}
default:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to generate a default empty case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This default case is triggered when the case channel would block. The chan read case will trigger if the Events channel is closed. This check is asserting that the channel returned by the Events method is blocking.

Fixes calling the Err method on SDK's Amazon Kinesis's SubscribeToShared
and Amazon S3's SelectObjectContent response EventStream members closing
the stream. This would cause unexpected read errors, or early
termination of the streams. Only the Close method of the streaming
members will close the streams.

Improved generated unit and integration tests for streaming API
operations.

Related to aws#2769
@jasdel jasdel force-pushed the issue-2769/S3SelectObjectStream branch from e135ecd to dddcc33 Compare October 10, 2019 18:29
@jasdel jasdel force-pushed the issue-2769/S3SelectObjectStream branch from dddcc33 to 108b9a1 Compare October 10, 2019 21:45
@@ -7379,6 +7379,8 @@ type SubscribeToShardEventStream struct {
// may result in resource leaks.
func (es *SubscribeToShardEventStream) Close() (err error) {
es.Reader.Close()
es.StreamCloser.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to handle the error, if there is one when trying to close a Reader or StreamCloser?

@jasdel jasdel merged commit 1d5d147 into aws:master Oct 21, 2019
@jasdel jasdel deleted the issue-2769/S3SelectObjectStream branch October 21, 2019 23:52
aws-sdk-go-automation pushed a commit that referenced this pull request Oct 22, 2019
===

### Service Client Updates
* `service/iotevents`: Updates service API and documentation
* `service/opsworkscm`: Updates service API and documentation
  * AWS OpsWorks for Chef Automate (OWCA) now allows customers to use a custom domain and respective certificate, for their AWS OpsWorks For Chef Automate servers. Customers can now provide a CustomDomain, CustomCertificate and CustomPrivateKey in CreateServer API to configure their Chef Automate servers with a custom domain and certificate.

### SDK Bugs
* `service/s3`,`service/kinesis`: Fix streaming APIs' Err method closing stream ([#2882](#2882))
  * Fixes calling the Err method on SDK's Amazon Kinesis's SubscribeToShared and Amazon S3's SelectObjectContent response EventStream members closing the stream. This would cause unexpected read errors, or early termination of the streams. Only the Close method of the streaming members will close the streams.
  * Related to [#2769](#2769)
aws-sdk-go-automation added a commit that referenced this pull request Oct 22, 2019
Release v1.25.17 (2019-10-22)
===

### Service Client Updates
* `service/iotevents`: Updates service API and documentation
* `service/opsworkscm`: Updates service API and documentation
  * AWS OpsWorks for Chef Automate (OWCA) now allows customers to use a custom domain and respective certificate, for their AWS OpsWorks For Chef Automate servers. Customers can now provide a CustomDomain, CustomCertificate and CustomPrivateKey in CreateServer API to configure their Chef Automate servers with a custom domain and certificate.

### SDK Bugs
* `service/s3`,`service/kinesis`: Fix streaming APIs' Err method closing stream ([#2882](#2882))
  * Fixes calling the Err method on SDK's Amazon Kinesis's SubscribeToShared and Amazon S3's SelectObjectContent response EventStream members closing the stream. This would cause unexpected read errors, or early termination of the streams. Only the Close method of the streaming members will close the streams.
  * Related to [#2769](#2769)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants