Breaking changes on 0.10 regarding Null field parsing on Maybe types #287

Closed
dmjio opened this Issue Sep 19, 2015 · 14 comments

Projects

None yet

8 participants

@dmjio
Contributor
dmjio commented Sep 19, 2015
-- 0.9
λ: newtype A = A { val :: Maybe String } deriving Show
(0.01 secs, 3,077,712 bytes)
λ: instance FromJSON A where parseJSON (Object o) = A <$> o .:? "val"
(0.00 secs, 1,032,488 bytes)
λ: eitherDecode "{ \"val\" : null }" :: Either String A
Right (A {val = Nothing})
(0.00 secs, 1,552,616 bytes)
-- 0.10
λ> newtype A = A { val :: Maybe String } deriving Show                                                                                                                            
(0.01 secs, 3,086,560 bytes)                                                                                                                                                      
λ>  instance FromJSON A where parseJSON (Object o) = A <$> o .:? "val"                                                                                                                                                                                                                                                                       
(0.01 secs, 2,069,808 bytes)                                                                                                                                                      
λ> eitherDecode "{ \"val\" : null }" :: Either String A                                                                                                                           
Left "Error in $.val: failed to parse field val: expected String, encountered Null"                                                                                               
(0.01 secs, 2,622,128 bytes)        

Is this intended behavior? Normally Maybe types would return Nothing in the presence of a Null field (Under 0.9).

This change causes the stripe-haskell project to fail 95% of tests. 😭

@dmjio
Contributor
dmjio commented Sep 19, 2015

Seems like this is the culprit, d0414be

Can we keep the existing operator (.:?) with 0.9.0 behavior, and as @gregwebs recommends, add a (.:??) operator for this new behavior ? ( as proposed by @lykahb in #83 )

@lykahb
lykahb commented Sep 19, 2015

You use a wrong operator. Field val has type Maybe String. In the return value of the (.:?) Maybe is related only to the presence of the key. The operator has never not handled nulls. The key exists so it tries to convert null into String.
If the key val is mandatory, you can use (.:). If it is optional, fmap join (o .:? "val") will use the right FromJSON instance.

@dmjio
Contributor
dmjio commented Sep 19, 2015

@lykahb I see, so previously fmap join (o .:? "val") was the default behavior (Null would be coerced into Nothing). Null and key existence were conflated. Now it's separate. Cool.

Maybe is related only to the presence of the key

In 0.10 yes, in 0.9 it could be related to the value too, right? Example below.

newtype A = A { val :: Maybe String } deriving Show
λ: instance FromJSON A where parseJSON (Object o) = A <$> o .: "val"
λ: eitherDecode "{ \"val\" : null }" :: Either String A
Right (A {val = Nothing})
@dmjio
Contributor
dmjio commented Sep 19, 2015

Here's a list of the old and new differences. http://lpaste.net/141286

@MaxGabriel

This is a major breaking change; I'd echo @gregwebs request that this be in the changelog or maybe use a different operator.

@olorin olorin added a commit to ambiata/github that referenced this issue Sep 30, 2015
@olorin olorin Add upper bound for aeson to avoid breaking changes in 0.10
aeson-0.10 changed `(.:?)` to give `empty` on nulls[0] (whereas it
previously gave `Nothing` on both nulls and missing keys). Documented
upstream[1] but no fix yet.

[0] bos/aeson#287
[1] phadej#121
08a7fee
@tolysz
tolysz commented Oct 11, 2015

Hi,
Just to add my 5 cents; (.??) is a part of the original proposal to add one extra constructor to the Value type (eg. https://github.com/tolysz/aeson/blob/MissingDefault2/examples/Possible.hs)
And really there is no other way around.
I've been keeping quiet but types Maybe and Maybe Maybe have different universal properties and sure you can hack around adding extra information on creation of From/ToJSON to cater for a difference but it will never be generic i.e. encodable in the type system by storing all information needed to present this data.

My Original proposal was:

toJson (Nothing :: Maybe a) =  Null
toJson (Just a :: Maybe a) =  toJson a

toJson (Nothing :: Maybe (Maybe a)) =  Missing -- extra constructor
toJson (Just Nothing :: Maybe (Maybe a)) =  Null 
toJson (Just (Just a) :: Maybe (Maybe a)) =  toJson a

-- or Equivalent
toJson (MissingData :: Possible a) =  Missing -- extra constructor
toJson (HaveHull :: Possible a) =  Null 
toJson ((HaveData a) :: Possible a) =  toJson a

This gives you some very nice properties: eg. the toJson a allows type a to decide on how it should present itself and thus easily build APIs eg. https://github.com/tolysz/google-api/blob/master/src/Network/Google/Api/Kinds.hs
where I do not need to specialise ListResponse to include HACK to show/hide fields it at times.

@potomak potomak added a commit to SumAll/haskell-canteven-metrics that referenced this issue Oct 14, 2015
@potomak potomak Make CarbonOptions keys required
Due to a new behavior of (.:?) in Data.Aeson >0.9 CarbonOptions
configuration values that are null fail to parse.

The (.:) behave in the same way in both Data.Aeson versions: it returns
Nothing if the value is null and it fails to parse if the key is not
present.

Usually configuration templates include all the keys and using (.:) seems
the appropriate choice.

See also bos/aeson#287.
ceb779d
@sapek
sapek commented Nov 11, 2015

@lykahb, this also seems to work in 0.10:

instance FromJSON A where parseJSON (Object o) = A <$> o .:? "val" .!= Nothing

Is this a valid option to get old behavior or do I need to use fmap join (o .:? "val")?

@lykahb
lykahb commented Nov 12, 2015

@sapek Yes, they should have the same behavior.

@rassie rassie referenced this issue in himura/twitter-types Nov 28, 2015
Closed

Not compatible with aeson-0.10 #36

@nikomi
nikomi commented Dec 14, 2015

I would +1 pull #288 - is anything happening with this thread?

@tolysz
tolysz commented Dec 15, 2015

This is sorted by: #326
I.e. the original question I mean. Simply one pattern got lost in the upgrade; Api does not change.

@nikomi
nikomi commented Dec 15, 2015

thanx for info!

Pull request #326 does not seem to be merged into master yet - any idea when this might be released?

@tolysz
tolysz commented Dec 15, 2015

I use stack; but I agree it prevents people from using 0.10 in the wild; and the sooner the better.
With this patch it simply builds all packages :)

@hvr
Collaborator
hvr commented Dec 20, 2015

@bos bump

@borsboom borsboom referenced this issue in commercialhaskell/stack Jan 8, 2016
Closed

stack --docker doesn't work with docker 1.9.1 #1570

@borsboom borsboom added a commit to commercialhaskell/stack that referenced this issue Jan 8, 2016
@borsboom borsboom Work around bos/aeson#287 (fixes #1570) b10bee1
@bos
Owner
bos commented Jan 20, 2016

I believe this should now be fixed.

@bos bos closed this Jan 20, 2016
@sapek sapek added a commit to Microsoft/bond that referenced this issue Jan 20, 2016
@sapek sapek Relax aeson upper bound
No longer necessary after bos/aeson#287 has been fixed.
8a89c44
@himura himura referenced this issue in himura/twitter-types Mar 11, 2016
Merged

Fix FromJSON instances for Status and User #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment