Don't throw on certain 404/409 responses

[tmenier]

Methods that Get, Patch, or Delete a thing by ID throw an exception on a 404 response (object does not exist). But in some cases (arguably most of them), this 404 is a convenient existence check in your logic flow and saves you an extra API call. In other words, it's not "exceptional", so perhaps it shouldn't throw. In the case of Get and Patch, getting a null value back seems like a more natural way to convey this. Deletes don't generally have a response body so would need to think on how best to handle that one.

A similar argument could be made for 409 (object already exists) responses with Create methods.

Important to note that any changes here could be breaking, so it would be best to make a decision on this ahead of 1.0.

[robertsoniv]

I agree with this for the most part; my own take:
GET Perhaps 204 No Content instead of a null response?
DELETE I say this should throw the same response as a successful delete (204 No content)
PATCH I think this should still respond with a 404

[tmenier]

@robertsoniv Thanks for the feedback. Keep in mind this is an SDK enhancement and not an API enhancement. The HTTP status is an API concern; SDK users should only be concerned with what that intuitively translates to from a code perspective (null vs. an exception). I'll definitely keep your thoughts in mind though - I'm thinking it might be best to make the "don't throw" behavior opt-in. Even though the SDK is technically still in beta, I know it's being used in a lot of production apps and I'm sensitive to breaking changes at this point.

[robertsoniv]

@tmenier Oh I see, you're just debating on if it should throw or not. Disregard!