-
Notifications
You must be signed in to change notification settings - Fork 50
Update client to PureScript 0.15.2 #278
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
Update client to PureScript 0.15.2 #278
Conversation
client/src/Try/API.purs
Outdated
_ -> | ||
Left "Tag must be one of: OtherError, CompilerErrors" | ||
j -> | ||
Left $ AtKey "tag" $ UnexpectedValue $ encodeJson j |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we preserve the detail in the error message using Tagged from codec-argonaut?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to do that. Can you clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant Named from Argonaut-codecs 😅 It lets you annotate an error, like:
Named “Message” $ UnexpectedValue …
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. It’s not really the right one, though, because it’s more for annotating structure. There’s not really a proper error type for this, other than maybe TypeMismatch. But I think it’s useful to show what the allowed constructors are in the error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why I don't like JsonDecodeError
. You can't provide error messages that have actionable "how to fix it" instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, how should I address this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I don't love it either. Wish I hadn't added it to argonaut proper.
One option is this:
AtKey "tag" $ TypeMismatch "Expected tag to be one of 'OtherError, CompilerErrors'"
but I'm fine leaving it with what you've written here, too. Your call :)
_ -> | ||
Left "Tag must be one of: OtherError, CompilerErrors" | ||
j -> | ||
Left $ AtKey "tag" $ UnexpectedValue $ encodeJson $ "- Expected a value of `OtherError` or `CompilerErrors` but got '" <> j <> "'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encodeJson
FTW! 😄
Description of the change
Fixes #264. Now that
0.15.2
is deployed to production, we can test the client out on the actual server vianpm run serve:production
.Checklist: