Skip to content
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

RPC tests are broken #265

Open
alexfmpe opened this issue May 2, 2024 · 0 comments
Open

RPC tests are broken #265

alexfmpe opened this issue May 2, 2024 · 0 comments

Comments

@alexfmpe
Copy link
Contributor

alexfmpe commented May 2, 2024

When trying to consume the output of /block I hit the following error

<interactive>: parseTimeOrError: no parse of "2024-05-02T01:47:36.734781Z"
CallStack (from HasCallStack):
  error, called at libraries/time/lib/Data/Time/Format/Parse.hs:124:15 in time-1.12.2:Data.Time.Format.Parse

this is triggered by

parseDiffTimeOrError :: String -> DiffTime
parseDiffTimeOrError = parseTimeOrError True defaultTimeLocale "%FT%T%QZ"

the root problem seems to be trying to parse a length of time when the field is actually a timestamp

ghci> parseTimeOrError True defaultTimeLocale "%FT%T%QZ" "2024-05-02T01:47:36.734781Z" :: UTCTime
2024-05-02 01:47:36.734781 UTC

ghci> parseTimeOrError True defaultTimeLocale "%FT%T%QZ" "2024-05-02T01:47:36.734781Z" :: DiffTime 
*** Exception: parseTimeOrError: no parse of "2024-05-02T01:47:36.734781Z"
CallStack (from HasCallStack):
  error, called at libraries/time/lib/Data/Time/Format/Parse.hs:124:15 in time-1.12.2:Data.Time.Format.Parse

Confusingly, the test suite suggests this endpoint is actually working properly

it "Can query /block and parse the result" $ const $ do
-- @NOTE: this defaults to latest block
result :: Either RPC.JsonRpcException RPC.ResultBlock <- try $ runRPC (RPC.block def)
result `shouldSatisfy` isRight

but if we add a print result we correctly get

Failures:
  (...)
  libraries/time/lib/Data/Time/Format/Parse.hs:124:15: 
  2) KVStore.Test.KV, Tendermint KV Store - via hs-tendermint-client, Can query /block and parse the result
       uncaught exception: ErrorCall
       parseTimeOrError: no parse of "2024-05-02T19:19:06.123194Z"
       CallStack (from HasCallStack):
         error, called at libraries/time/lib/Data/Time/Format/Parse.hs:124:15 in time-1.12.2:Data.Time.Format.Parse

it seems like we're not fully forcing the Success value in

Left rpcError -> throwM $ CallException rpcError
Right resultValue ->
case fromJSON resultValue of
Error err -> throwM $ ParsingException err
Success result -> pure result

so this blows up in client code eventually.

The tests definitely should be dealing with the normal form of the value but I'm not sure what the lib should be doing, given this can only be a problem when using unsafe code.

The current behavior preserves laziness but leads to unsafe code having unexpected behavior, which suggests a need for a strict option, even if less performant when the entire value isn't needed. In which case, that could replace the current behavior or exist alongside it.

To avoid duplicating every endpoint, maybe results could be wrapped in a newtype that users then need to choose to "run" lazily or strictly so a conscious choice is made every time?

EDIT: corrected the source of the excessive laziness issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant