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

AmazonSQSClient.DeleteQueueAsync should not throw exception when deleted non-existent queues #3018

Closed
2 tasks
Coder3333 opened this issue Aug 2, 2023 · 5 comments
Labels
feature-request A feature should be added or improved. module/sdk-generated needs-review

Comments

@Coder3333
Copy link

Describe the feature

AmazonSQSClient.DeleteQueueAsync throws the following exception if you try to use it to delete a queue that was previously deleted. I am requesting that this scenario would no longer throw an exception. I do not see any benefit in DeleteQueueAsync throwing an exception for a queue that does not exist, since the goal of the api is to get rid of a queue.

Amazon.SQS.AmazonSQSException: The specified queue does not exist for this wsdl version.
---> Amazon.Runtime.Internal.HttpErrorResponseException: Exception of type 'Amazon.Runtime.Internal.HttpErrorResponseException' was thrown.
at Amazon.Runtime.HttpWebRequestMessage.GetResponseAsync(CancellationToken cancellationToken)
at Amazon.Runtime.Internal.HttpHandler1.InvokeAsync[T](IExecutionContext executionContext) at Amazon.Runtime.Internal.Unmarshaller.InvokeAsync[T](IExecutionContext executionContext) at Amazon.SQS.Internal.ValidationResponseHandler.InvokeAsync[T](IExecutionContext executionContext) at Amazon.Runtime.Internal.ErrorHandler.InvokeAsync[T](IExecutionContext executionContext) --- End of inner exception stack trace --- at Amazon.Runtime.Internal.HttpErrorResponseExceptionHandler.HandleExceptionStream(IRequestContext requestContext, IWebResponseData httpErrorResponse, HttpErrorResponseException exception, Stream responseStream) at Amazon.Runtime.Internal.HttpErrorResponseExceptionHandler.HandleExceptionAsync(IExecutionContext executionContext, HttpErrorResponseException exception) at Amazon.Runtime.Internal.ExceptionHandler1.HandleAsync(IExecutionContext executionContext, Exception exception)
at Amazon.Runtime.Internal.ErrorHandler.ProcessExceptionAsync(IExecutionContext executionContext, Exception exception)
at Amazon.Runtime.Internal.ErrorHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.Signer.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.EndpointDiscoveryHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.EndpointDiscoveryHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.CredentialsRetriever.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.RetryHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.RetryHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.ErrorCallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
at Amazon.Runtime.Internal.MetricsHandler.InvokeAsync[T](IExecutionContext executionContext)

Use Case

The reason this is such an issue is that AmazonSQSClient.ListQueuesAsync will return queues that were already deleted, which means that my application has no way to know which queues actually exist prior to calling DeleteQueueAsync. To make things more complicated, I cannot simply wrap the call to DeleteQueueAsync in a catch block and blindly eat all exceptions, as I would want to react differently if the api failed for a networking issue.

Proposed Solution

If AmazonSQSClient.DeleteQueueAsync cannot find the queue being requested, do not throw an exception. Update DeleteQueueResponse to have a flag indicating whether the requested queue was found.

Other Information

I already requested that AmazonSQSClient.ListQueuesAsync not return deleted queues, but was told that was not going to happen. I believe simply having AmazonSQSClient.DeleteQueueAsync not throw an exception for this case is a better solution to the problem.

#2451

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

AWS .NET SDK and/or Package version used

AWSSDK.SQS 3.7.100.30

Targeted .NET Platform

.Net 6

Operating System and version

Windows 10

@Coder3333 Coder3333 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 2, 2023
@ashishdhingra
Copy link
Contributor

@Coder3333 Thanks for submitting the feature request. Per service API documentation at DeleteQueue, QueueDoesNotExist error is thrown if this operation is invoked for a non-existent queue.

The workaround in discussion #2451 was proposed due to limitation of the service API, not in the SDK.

For logic in AWS SDK(s) which is auto-generated (as mentioned in discussion #2451), per design it would return any exception returned by the service to the calling code. The response object DeleteQueueResponse shape is derived from the service model shared by the service team. Hence, we could not simply add a new flag since it would be overwritten when the class is auto-generated by service models in next build/push.

Instead of wrapping the calls to DeleteQueueAsync() in try{}catch{} block, you may implement a utility method which can return the status of the call (refer similar utility method AmazonS3Util.DoesS3BucketExistV2Async() for S3).

Thanks,
Ashish

@ashishdhingra ashishdhingra 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 Aug 2, 2023
@Coder3333
Copy link
Author

@ashishdhingra , automatically waiting 60 seconds before any call to List, just to make sure any previous calls to Delete have completed is not reasonable at all. Also, it does not make a lot of sense for me to manage a helper function that ignores specific exceptions when I do not know what the full list of ignorable exceptions is, nor do I know when those change. It would make a lot more sense for AWS to manage that helper function, since they intimately know the return codes from the service. The exception doesn't even provide an enumeration that I can inspect to determine the reason.

@ashishdhingra
Copy link
Contributor

@ashishdhingra , automatically waiting 60 seconds before any call to List, just to make sure any previous calls to Delete have completed is not reasonable at all. Also, it does not make a lot of sense for me to manage a helper function that ignores specific exceptions when I do not know what the full list of ignorable exceptions is, nor do I know when those change. It would make a lot more sense for AWS to manage that helper function, since they intimately know the return codes from the service. The exception doesn't even provide an enumeration that I can inspect to determine the reason.

@Coder3333 Thanks for your reply. I would review this with team and post any updates here.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Aug 3, 2023
@normj
Copy link
Member

normj commented Aug 18, 2023

Hi @Coder3333, I agree with you that have to do control flow by exceptions is not ideal but utility method from the SDK method that essentially buries the exception isn't practical for us. The SDK doesn't know the intent and can't tell the difference if this is a not found because of an eventual consistent issue between the list call or the queue name is just invalid. Then we have a consistence issue with the SDK itself because there are 1000+ delete methods across the SDK, should they all of a utility method? But we don't have any meta data from our service definitions that we use to generate the service client to inform our generator if and how to generate the utility method.

@normj normj closed this as completed Aug 18, 2023
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. module/sdk-generated needs-review
Projects
None yet
Development

No branches or pull requests

3 participants