soto-project / soto

Swift SDK for AWS that works on Linux, macOS and iOS
https://soto.codes
Apache License 2.0
875 stars 83 forks source link

SQS `sendMessage` yields `Precondition failed` error. #578

Closed seopyoon closed 2 years ago

seopyoon commented 2 years ago

Describe the bug Upon sending message using SotoSQS, after successfully sending message to SQS queue, it errors with Precondition failed error.

To Reproduce Steps to reproduce the behavior:

  1. Clone soto-vapor-test (https://github.com/soto-project/soto-vapor-test)
  2. Create SqsController
    
    import SotoSQS
    import Vapor

class SqsController { func sendMessage(_ req: Request) -> EventLoopFuture { let sqsUrl = "(mySqsUrl)" let sqsMessageReq = SQS.SendMessageRequest(messageBody: "{\"key\": \"value\"}", queueUrl: sqsUrl) return req.aws.sqs.sendMessage(sqsMessageReq).map { output in return output.messageId ?? "nil" }.flatMapErrorThrowing { error in if let error = error as? AWSErrorType { throw Abort(.badRequest, reason: error.description) } throw error } } }

3. Attach route to sqs 
``` swift
let sqsController = SqsController()
app.post("sqs", use: sqsController.sendMessage)
  1. Error. soto-vapor-test-main-gldreccdrpihrcdodobgfdksuevy/SourcePackages/checkouts/swift-nio/Sources/NIOCore/ChannelPipeline.swift:1765: Precondition failed

A clear and concise description of what you expected to happen.

Setup (please complete the following information):

Additional context The message is sent successfully. But it error afterwards.

adam-fowler commented 2 years ago

This is because the SES call can be running on a separate EventLoop. You need to add a .hop(to: req.eventLoop) after the flatMapErrorThrowing to return to the correct EventLoop.

That's correct isn't it @0xTim

seopyoon commented 2 years ago

@adam-fowler That fixed it. Could you elaborate on why that is, in a little bit more detail?

adam-fowler commented 2 years ago

Hi, Swift NIO requires any work kicked off from a channel returns an EventLoopFuture scheduled on the same EventLoop as the channel runs on.

Soto cannot guarantee it is going to run on the current EventLoop as the underlying HTTP client does not guarantee it. The HTTP client will favour an existing connection over creating a new connection. This might mean it has to run on a different EventLoop which that connection runs on.

An alternative to adding a .hop at the end of your call is to pass the event loop into the SES.sendMessage call. This is a hint to the HTTP client on where it wants the work to be done, and it will ensure the EventLoopFuture returned is scheduled on the correct EventLoop. This is probably a preferable method. (I didn't initially suggest this because I needed to verify with the authors of AsyncHTTPClient it would return an EventLoopFuture scheduled on the correct EventLoop).

req.aws.sqs.sendMessage(sqsMessageReq, eventLoop: req.eventLoop)
seopyoon commented 2 years ago

Thanks!