Eliminate (really unsafe) record fields on data types with variants.

[dbp]

I just converted an application from the previous stripe library, and in general things have been good, but I have some pretty serious concerns about safety. In particular, I just saw an error in production due to this problem. Essentially, record field accessors on data types with variants are really unsafe. The particular one that I hit was on Customer - I was converting code, and the other stripe library had only one Customer variant, and had a (perfectly safe and good) customerId record field. So the code compiled, and then at runtime, it got passed a DeletedCustomer, and exploded.

To be honest, I think having these be variants at all is pretty bizarre. I would much prefer if DeletedCustomer was it's own type, and if an endpoint can return either, then make it be Either. Having Customer sometimes not really be customer is odd.

[dmjio]

There is a nuance (specifically for customers, and only customers), that retrieving deleted customer records don't 404, they stick around. I agree that these partial functions need to be removed and the current solution is not ideal. @stepkut's changes still need to be merged, and I believe they remedy this situation. It's been hard to get a breather from work, and the merge is huge. I apologize for this, and plan to make the changes asap.

[dmjio]

@dbp, thought of a solution to this. Then make all customer retrieval functions use this.

{-# LANGUAGE FlexibleInstances #-}                                                                                                                                                
import           Control.Applicative                                                                                                                                              
import           Data.Aeson                                                                                                                                                                                                                                                                                                                        

instance FromJSON (Either DeletedCustomer Customer) where                                                                                                                                       
  parseJSON (Object o) = do                                                                                                                                                       
    result <- o .: "deleted"                                                                                                                                                    
    case result of                                                                                                                                                                
      True  ->  Left <$> (DeletedCustomer <$> parseJSON o)
      False -> Right <$> (Customer <$> parseJSON o)                                                                                                                       

[dbp]

@dmjio That sounds great.

[dmjio]

This issue is a bit of a pickle. I've implemented:

newtype CustomerResult = CustomerResult (Either DeletedCustomer Customer)
  deriving (LotsOfThings)

All the tests pass except for one. For some reason removing a discount on a customer actually deletes the customer. Still pondering...

[bitemyapp]

For some reason removing a discount on a customer actually deletes the customer

wot

[dmjio]

Yea, unsure why. Maybe some actions are being called out of order.