[ 
https://issues.apache.org/jira/browse/THRIFT-908?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13485783#comment-13485783
 ] 

Spiridon Eliopoulos commented on THRIFT-908:
--------------------------------------------

The reason that Java and C++ can treat errors in this way is because they both 
have a notion of a null value that inhabits all class types. A 'new Car("red")' 
and null are both Cars, but if you try to beep() the latter, the consequences 
won't be covered by your insurance.

null is treated differently in Haskell though the use of the Maybe data type. 
There's no conflation of a null value with all other values in a type,  
allowing your Haskell compiler to statically verify that you never attempt to 
manipulate a value that you in fact don't have (through inspecting their static 
types). In theory this should mean that runtime errors like 
NullPointerExceptions are impossible for your program to produce. That's the 
goal, and that's true (at least it's more true) when 'undefined' does not 
appear in your program. But, the use of 'undefined' in this patch introduces 
the very behavior, which I believe should be avoided at all costs.

This isn't to say that Haskell code shouldn't throw errors. It's about when and 
why they throw errors. For example, I'd prefer that this change not produce 
records with fields set to 'undefined', since that error could be triggered at 
any time, not necessarily right when the client's RPC returns (for example). A 
better way to do it would be to read the entire RPC call or return value, 
verify that all the required fields are present, and raise an error at that 
point indicating that there's some sort of client/server schema mismatch.
                
> Make required types actually required by the Haskell type system
> ----------------------------------------------------------------
>
>                 Key: THRIFT-908
>                 URL: https://issues.apache.org/jira/browse/THRIFT-908
>             Project: Thrift
>          Issue Type: Bug
>          Components: Haskell - Compiler, Haskell - Library
>    Affects Versions: 0.6
>         Environment: ghc 6.12.3 on Mac OS X 10.6 (Haskell Platform 2010.2)
> Darwin -- 10.4.0 Darwin Kernel Version 10.4.0: Fri Apr 23 18:28:53 PDT 2010; 
> root:xnu-1504.7.4~1/RELEASE_I386 i386
> The Glorious Glasgow Haskell Compilation System, version 6.12.3
>            Reporter: Christian Lavoie
>            Assignee: Christian Lavoie
>         Attachments: v1-thrift-make-required-fields-required.patch
>
>   Original Estimate: 6h
>  Remaining Estimate: 6h
>
> Currently Haskell bindings consider all struct fields to be optional by 
> having all struct fields be "Maybe $field". This patch removes that Maybe for 
> required fields.
> THIS BREAKS EXISTING CODE.
> It's also probably incomplete -- I haven't looked too carefully yet at what 
> happens on the server side if it tries to parse out an incomplete struct 
> definition:
> {noformat}
> struct myStruct {
>   1: required byte foo,
> }
> {noformat}
> became after some code changes
> {noformat}
> struct myStruct {
>   1: optional byte foo,
> }
> {noformat}
> And code generated for the first version is asked to read a struct from the 
> second version where foo is missing.
> I suspect after this patch the Haskell server commits hara-kiri trying to 
> evaluate {{undefined}} _outside_ of the thrift generated code (so the client 
> could would receive an error for something that the thrift generated code 
> should have noticed as wrong). This is clearly inadequate.
> Consider this bug a thought experiment / request for comments until I 
> validate that the patch is complete. It will still break existing code.
> First and foremost:
> # Thoughts?
> # What would be an acceptable way to handle the case described above for 
> other Haskell users? Break the current interface some more by returning some 
> error type (Either $struct $errMessags?). Throw a Haskell exception?
> # How do the other bindings handle parsing invalid data off the network?
> # The generator and generated code is fairly painful to read. Would the 
> powers that be welcome a cleanup patch that moves as much of the 
> t_hs_generator.cpp code to a boost::format-style system that uses template 
> strings that look like Haskell code, and reformats the output to be indented, 
> proper style Haskell code as much as possible?

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to