New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime error when decoding Json #624

Closed
patrik-piskay opened this Issue May 26, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@patrik-piskay

patrik-piskay commented May 26, 2016

I am getting Uncaught TypeError: Cannot read property 'tag' of undefined (https://github.com/elm-lang/core/blob/32b8fd9b3f2f1f6ef2c1d3a7914427e7a646770c/src/Native/Json.js#L313) when decoding Json.

Repo with minimum code required to replicate the issue with Json response hardcoded here

Main parts:

import Json.Decode as Json exposing ((:=))

type alias TweetModel =
    { id : Float
    , text : String
    , retweetedStatus : Maybe RetweetedStatus
    }

type RetweetedStatus = RetweetedStatus TweetModel

decodeTweets : Json.Decoder (List TweetModel)
decodeTweets =
    tweetDetails |> Json.list

tweetDetails : Json.Decoder TweetModel
tweetDetails =
    Json.object3 TweetModel
        ("id" := Json.float)
        ("text" := Json.string)
        (Json.maybe ("retweeted_status" := tweetDetails |> Json.map RetweetedStatus))

The issue seems to be related to the RetweetedStatus type and and its usage in decoder. Without it, everything works fine.

In twitter response, in each tweet record there may or may not be a retweeted_status key, hence the Json.Decode.maybe

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender May 26, 2016

Contributor

The problem is the recursive dependency of tweetDetails on itself. This is currently not supported, the issue that needs to be fixed in order to support it is this: elm/compiler#873. It's well known, so I'm closing this issue here.

To work around the issue currently, you should use lazy and force from http://package.elm-lang.org/packages/elm-lang/lazy. For more details, check out some of the issues mentioning "JSON" or "decode" listed over at elm/compiler#873.

Contributor

jvoigtlaender commented May 26, 2016

The problem is the recursive dependency of tweetDetails on itself. This is currently not supported, the issue that needs to be fixed in order to support it is this: elm/compiler#873. It's well known, so I'm closing this issue here.

To work around the issue currently, you should use lazy and force from http://package.elm-lang.org/packages/elm-lang/lazy. For more details, check out some of the issues mentioning "JSON" or "decode" listed over at elm/compiler#873.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender May 26, 2016

Contributor

Something like this should work:

decodeTweets : Json.Decoder (List TweetModel)
decodeTweets =
    force tweetDetails |> Json.list

tweetDetails : Lazy (Json.Decoder TweetModel)
tweetDetails = lazy <| \_ ->
    Json.object3 TweetModel
        ("id" := Json.float)
        ("text" := Json.string)
        (Json.maybe ("retweeted_status" := force tweetDetails |> Json.map RetweetedStatus))
Contributor

jvoigtlaender commented May 26, 2016

Something like this should work:

decodeTweets : Json.Decoder (List TweetModel)
decodeTweets =
    force tweetDetails |> Json.list

tweetDetails : Lazy (Json.Decoder TweetModel)
tweetDetails = lazy <| \_ ->
    Json.object3 TweetModel
        ("id" := Json.float)
        ("text" := Json.string)
        (Json.maybe ("retweeted_status" := force tweetDetails |> Json.map RetweetedStatus))
@patrik-piskay

This comment has been minimized.

Show comment
Hide comment

patrik-piskay commented May 26, 2016

Thanks @jvoigtlaender 👍

@patrik-piskay

This comment has been minimized.

Show comment
Hide comment
@patrik-piskay

patrik-piskay May 26, 2016

I used this approach and now I am getting Maximum call stack size exceeded error. I will probably fix this by refactoring the code and use another decoder for retweet as there should not be any retweeted_status inside retweeted_status. I just wanted to reuse one decoder recursively as the rest of the structure is the same.

But what I am curious about, I got to this code by following this hint about recursive types/type aliases and if I would be expecting a response Json of shape used in example there (messages that can have response messages and those can have response messages and so on), what would be the working approach there? I would need to use some kind of recursive decoder there like I was doing with the tweets and lazy approach just produces a different runtime error.

Thanks for pointing me to the right direction, still new to Elm :)

patrik-piskay commented May 26, 2016

I used this approach and now I am getting Maximum call stack size exceeded error. I will probably fix this by refactoring the code and use another decoder for retweet as there should not be any retweeted_status inside retweeted_status. I just wanted to reuse one decoder recursively as the rest of the structure is the same.

But what I am curious about, I got to this code by following this hint about recursive types/type aliases and if I would be expecting a response Json of shape used in example there (messages that can have response messages and those can have response messages and so on), what would be the working approach there? I would need to use some kind of recursive decoder there like I was doing with the tweets and lazy approach just produces a different runtime error.

Thanks for pointing me to the right direction, still new to Elm :)

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender May 26, 2016

Contributor

... what would be the working approach there? I would need to use some kind of recursive decoder there like I was doing with the tweets and lazy approach just produces a different runtime error.

The working approach should be the one using lazy and force. Other people, in some of the GitHub issues I mentioned earlier, the ones listed at elm/compiler#873 that are about JSON decoding, have successfully used that approach to deal with decoding recursive structures, such as decoding JSON data that corresponds to types like this:

type Tree = Leaf Int | Node Tree Tree

So this approach is not unworking per se. Why it does not work in your specific case is difficult to diagnose without having an actual http://sscce.org/, including a call with a concrete JSON input that you expect to be able to decode but get the runtime error on.

Contributor

jvoigtlaender commented May 26, 2016

... what would be the working approach there? I would need to use some kind of recursive decoder there like I was doing with the tweets and lazy approach just produces a different runtime error.

The working approach should be the one using lazy and force. Other people, in some of the GitHub issues I mentioned earlier, the ones listed at elm/compiler#873 that are about JSON decoding, have successfully used that approach to deal with decoding recursive structures, such as decoding JSON data that corresponds to types like this:

type Tree = Leaf Int | Node Tree Tree

So this approach is not unworking per se. Why it does not work in your specific case is difficult to diagnose without having an actual http://sscce.org/, including a call with a concrete JSON input that you expect to be able to decode but get the runtime error on.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender May 27, 2016

Contributor

Coming back to this since it was pointed out in elm/elm-lang.org#578 that lazy from elm-lang/lazy is overkill. So the following should work at http://elm-lang.org/try, and handle your case:

decodeTweets : Json.Decoder (List TweetModel)
decodeTweets =
    lazy (\_ -> tweetDetails) |> Json.list

tweetDetails : Json.Decoder TweetModel
tweetDetails = 
    Json.object3 TweetModel
        ("id" := Json.float)
        ("text" := Json.string)
        (Json.maybe ("retweeted_status" := lazy (\_ -> tweetDetails) |> Json.map RetweetedStatus))

lazy : (() -> Json.Decoder a) -> Json.Decoder a
lazy thunk =
    Json.customDecoder Json.value
        (\js -> Json.decodeValue (thunk ()) js)

It's possible the lazy-call in decodeTweets is even superfluous.

Contributor

jvoigtlaender commented May 27, 2016

Coming back to this since it was pointed out in elm/elm-lang.org#578 that lazy from elm-lang/lazy is overkill. So the following should work at http://elm-lang.org/try, and handle your case:

decodeTweets : Json.Decoder (List TweetModel)
decodeTweets =
    lazy (\_ -> tweetDetails) |> Json.list

tweetDetails : Json.Decoder TweetModel
tweetDetails = 
    Json.object3 TweetModel
        ("id" := Json.float)
        ("text" := Json.string)
        (Json.maybe ("retweeted_status" := lazy (\_ -> tweetDetails) |> Json.map RetweetedStatus))

lazy : (() -> Json.Decoder a) -> Json.Decoder a
lazy thunk =
    Json.customDecoder Json.value
        (\js -> Json.decodeValue (thunk ()) js)

It's possible the lazy-call in decodeTweets is even superfluous.

@patrik-piskay

This comment has been minimized.

Show comment
Hide comment
@patrik-piskay

patrik-piskay commented May 27, 2016

Thanks 👍

@carltongibson

This comment has been minimized.

Show comment
Hide comment
@carltongibson

carltongibson Jul 27, 2016

I found this via the same path as @patrik-piskay — via working through recursive type aliases advice and then hitting the runtime error.

For me using lazy from the elm-community/json-extras package was super simple.

(I leave the note for the future. 🙂)

carltongibson commented Jul 27, 2016

I found this via the same path as @patrik-piskay — via working through recursive type aliases advice and then hitting the runtime error.

For me using lazy from the elm-community/json-extras package was super simple.

(I leave the note for the future. 🙂)

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jul 28, 2016

Contributor

Yes, lazy from that package is like the lazy from my comment above. That package is not available at http://elm-lang.org/try, though, so when trying out stuff there (as above) an explicit definition of lazy is needed.

Contributor

jvoigtlaender commented Jul 28, 2016

Yes, lazy from that package is like the lazy from my comment above. That package is not available at http://elm-lang.org/try, though, so when trying out stuff there (as above) an explicit definition of lazy is needed.

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