s3 fail detector

[xtat]

Not sure how you'd like to do this! Ran into s3 fails on a production app!

[domenic]

I agree that all the higher-level API functions should check for 2xx or 3xx status codes, and if not, return an error. Perhaps they should even read in the response body as the error message.

This particular implementation isn't great, as it only applies to putStream (which needs an overhaul anyway), doesn't account for other 2xx or 3xx successes, and throws away the failure information. But the impetus is sound, so I'll leave it open as a reminder.

[domenic]

Here is how Dynode implements it:

/~https://github.com/Wantworthy/dynode/blob/c29ab5c5a110c86f82329f49289647cd7a057bc9/lib/dynode/request.js#L41-54

They have the luxury of getting JSON back instead of XML, of course. Also we'd have to be sure not to just check for 200, but for 2xx as well (e.g. 201 on PUT).

[tim-kos]

It would also be cool to be able to provide more events on the http request, like on('close') and on('clientError').

We also got S3 fails in production and since these events are not thrown/handled, we ran into an uncaught exception. : /

[domenic]

@tim-kos: I don't think http.ClientRequest emits "clientError", and "close" is not an error condition, so I don't see why we would listen for that...

[tim-kos]

Sorry, I meant on('error').

[domenic]

@tim-kos We currently do have listeners for req.on("error"):

/~https://github.com/LearnBoost/knox/blob/18afc26fa586ba2df813019de5cc7efb249deda9/lib/client.js#L35

[tim-kos]

Okay thank you. I missed those.

[domenic]

Ok, just let me know if you do spot anything Knox can do to help with your unhandled exceptions :)

On Aug 2, 2012, at 10:30, "Tim Koschuetzki" reply@reply.github.com wrote:

Okay thank you. I missed those.

---

Reply to this email directly or view it on GitHub:
#59 (comment)

[boutell]

Shouldn't the 307 code be implemented by trying again automatically at the suggested URL?

[domenic]

@boutell yes, that is a feature we would like to add, see #66.

[domenic]

Closed in favor of #114