Errors

[GetContented]

Hi @martinklepsch I'm about to implement something around errors (because I need it on s3-beam in my app). Are you happy with the idea of passing in a chan on the opts map, which, if present will have a map of error-message and error-code put on it? (something like {:error-message "" :error-code 0} I'm imagining at this stage)?

I'll go ahead with that anyway, let me know if you've got another preference.

[martinklepsch]

Maybe errors could be sent to the same channel as successful uploads and messages just contain some different data?

Really welcoming any efforts toward error handling! :)

[GetContented]

I did consider that, but isn't that kind of a breaking change? Your current implementation puts the file back into the done channel, I think, so the contract you have is that "anything" received on that channel is indication that the upload was successful.

However, if breaking changes aren't too much of a concern, using the file itself as an indicator is probably less than ideal. I wonder if we could wrap the file in a map, and that way we could mark it with an identifier that we could respond with... the reason being that you have async uploads (concurrency of 3, I think), then they won't necessarily succeed in the order that you've put them up in. ({:file # :identifier "some-identifier"} then on response, {:identifier "some-identifier" :response-code 200} (or the error code and error message))?

[martinklepsch]

I'm not concerned about breaking changes as long as we're sub 1.0.

So if right now we're sending back the plain location of the file I think it would be a good idea to make that a bit more extendable, I.e. a map.

An identifier makes sense, probably that should be the key on S3 — see #7 and #16 for that.

[GetContented]

Hm. Ok. So at the moment there's no way to specify the object key? That's a bit of a deal breaker for me. Would need to have that fixed ASAP. Can we merge bensu's PR in? I don't really understand what the key-fn does... personally I'd like to be able to specify the key myself... is this the file-name passed to the server side sign function?

I don't use the clojure / ring server. I'm unfortunately stuck on Rails for the next few months at least... so I've written my own backend sign function...

[GetContented]

I think I understand the intent of the key-fn now... but I'd still like the option of passing in an object-key to the map as first preference over the key... is bensu's branch ready to be merged? Is there a reason you haven't merged it already, @martinklepsch ?

Update: no need to respond to this.

[GetContented]

Ok, so here's my plan:

1. make s3-pipe put a response map of interpreted data from S3 onto its report-chan, instead of just the file object or location.
2. make it so the s3-pipe can have either a map or a file put onto its upload-file chan: in the case of the map, it will have a :file key that contains the file, and will later be adjusted to allow an "object key" key to be passed through as well. Add an identifier key to this map for later passing back on the response channel - for identification by the user code that handles the response
3. make so that s3-pipe (and possibly uploader) handles errors - such as http errors, and response codes from AWS, and reports these back on the response map
4. make so that s3-pipe can have a key-fn passed to it which will be used to generate file keys before signing
5. make it so that the upload-file chan’s map can have an object key which will be used for the file key before signing

Do you want these all as one PR? It seems like it might be a lot of changes to review all at once.

[GetContented]

@martinklepsch ok, I've finished this so I'll put up a PR. No response from you yet as to how you'd like to proceed, but, well, you can check it out and see what you think.

[GetContented]

Ok @martinklepsch I've created https://github.com/martinklepsch/s3-beam/pull/21 which should hopefully supercede https://github.com/martinklepsch/s3-beam/pull/7 and https://github.com/martinklepsch/s3-beam/pull/16 if I've done it correctly.

Most importantly, it now handles error that happen at either the key-signing server, or the AWS server. Also of interest:

• It handles object naming on the client (the right place for it, I think) - either automatically with a key-fn or "manually" when sending the file to the upload channel (useful if you need to get an id for a record that you also want to put into the object key).
• It lets you specify an identifier (which is handy to identify files before and after upload if you use the new key-fn to generate your object keys).
• It lets you specify an object key when putting your file to the upload channel.
• The upload channel can now take either a map or just a file if you want to do it simply.
• The response now comes back as a map on the reporting channel, with various error messages and server responses in it, which is invaluable for debugging and reporting.
• The functions have docstrings, so hopefully they're reasonably self-explanatory.

It could deal with some additional documentation and adjustment to the readme, which I'd be happy to write if you'd like.

I'm using this in production now, so I'm pretty confident it works ok. Still no tests, though! (gulp)

I'll leave this open until there's been some more movement on it.

[martinklepsch]

Closed via #24