Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-2343

Golang - Return a single error for all exceptions instead of multiple return values

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.9.1
    • 0.9.2
    • Go - Compiler
    • None
    • Patch Available

    Description

      Currently the Thrift compiler for Go generates some very ugly (by Go's standard) code for method calls. This is because of the fact that Go does not have exceptions.

      My proposal is to change the way the exceptions are handled in Go, instead of returning each exception explicitly as a return value, instead return the exceptions as a single error return. This has another benefit that Go servers will no longer be able to 'throw' multiple exceptions, previously you could assign an error to all the defined exceptions of a method which shouldn't be possible.

      This change relies on the fact that the exceptions generated by Thrift implement the Error interface, which they now do (in 0.9.2).

      It has the downside that the exceptions thrown to a client calling a thrift method are no longer immediately visible. But this is the same way the Go standard library handles it so it shouldn't be a problem.

      It changes the generated interface so it will break backwards compatibility, but as 0.9.2 is already breaking backwards compatibility this shouldn't be a huge problem, but should be considered.

      The proposed changes change the Processor methods for each method, doing a type switch against all the defined exceptions for the method. The fallthrough case is the same INTERNAL_SERVER_ERROR exception.

      Please consider this a RFC.

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            jensg Jens Geyer
            zariel Chris Bannister
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment