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

S3Client.download fails silently on 404 #1117

Closed
eyalfa opened this issue Jul 19, 2018 · 5 comments
Closed

S3Client.download fails silently on 404 #1117

eyalfa opened this issue Jul 19, 2018 · 5 comments
Labels
Milestone

Comments

@eyalfa
Copy link
Contributor

eyalfa commented Jul 19, 2018

when attempting to download a missing key from s3, the returned ObjectMetaData future completes successfully and the returned Source completes without emitting any data.

I suspect the root cause is in akka.stream.alpakka.s3.impl.S3Stream#download:

def download(s3Location: S3Location,
               range: Option[ByteRange],
               sse: Option[ServerSideEncryption]): (Source[ByteString, NotUsed], Future[ObjectMetadata]) = {
    import mat.executionContext
    val s3Headers = S3Headers(sse.fold[Seq[HttpHeader]](Seq.empty) { _.headersFor(GetObject) })
    val future = request(s3Location, rangeOption = range, s3Headers = s3Headers)
    val source = Source
      .fromFuture(future.flatMap(entityForSuccess))
      .map(_.dataBytes)
      .flatMapConcat(identity)
    val meta = future.map(resp ⇒ computeMetaData(resp.headers, resp.entity))
    (source, meta)
  }

as you can see the initial future is an HttpResponse future, in this case it has a 404 status, it's first being flatMapped to create the source and later mapped to created the metadata object.
the flatMap internally checks the response status while map fails to do so.

in my use-case I pass this source to play's Ok.sendEntity (play.api.mvc.Results.Status#sendEntity) so I don't control on the way it is being consumed and how are failures handled, I'd prefer being signaled about the missing key (via a failure or 'Option[ObjectMetadata]') so my program can behave accordingly.

@2m
Copy link
Member

2m commented Jul 19, 2018

Interesting. The Future is flatmapped with entityForSuccess which checks the response status code for success which is false for 404s and eventually throws. There is something more going on.

@eyalfa
Copy link
Contributor Author

eyalfa commented Jul 19, 2018

@2m , thanks for the quick reply.
please notice that the flatMap using entityForSuccess applies only for the returned source, the meta future is produed by mapping over the original response future and it does not check the status.
assuming I'd find some time in the weakend, I might be able to introduce a failing test and a fix PR.

@anikiforovopensource
Copy link
Contributor

@eyalfa I've seen more silent failures with the alpakka-s3 connector.

When the processing is slow, and the s3 connector is back-pressured, it causes the source to hang irrecoverably. The connector wont throw an exception and wont emit any more data downstream. Eventually this causes the entire application to hang, as it cannot make any progress, and wont fail so it can be restarted.

@eyalfa
Copy link
Contributor Author

eyalfa commented Jul 21, 2018

@lexn82 , can u reproduce this behavior in a test?
This is quite surprising as the underlying stream is implemented by akka-http which is supposed to behave properly in case of backpreasure.

@2m 2m added this to the 0.21 milestone Jul 23, 2018
@2m 2m added the p:aws-s3 label Jul 23, 2018
@2m
Copy link
Member

2m commented Jul 23, 2018

Fixed in #1122

@2m 2m closed this as completed Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants