Change client to return instances of our classes.

[markdoliner-doma]

This changes the client code generated for --target thrift-server so that it returns instances of the generated thrift classes instead of plain objects. I believe the client code generated for --target apache already behaves this way.

One advantage is this allows instanceof to be used on Thrift responses, which makes exceptions distinguishable from each other using instanceof. It would also increase the usefulness of having exception classes derive from Error.

The change itself is small and easy. The compiler already generates "MyEndpoint__Result" classes that have a read() function that instantiates the class, I just needed to call it from the appropriate place in the Client class.

To review this change it'll be helpful if you're familiar with the various classes that are generated. I'll refrain from trying to describe it here--you're better off looking at some generated code on your own. Here's a diff showing how this change affects a generated ping function. This is in the Client class:

     public ping(context?: Context): Promise<string> {
         const writer: thrift.TTransport = new this.transport();
         const output: thrift.TProtocol = new this.protocol(writer);
         output.writeMessageBegin("ping", thrift.MessageType.CALL, this.incrementRequestId());
         const args: IPing__ArgsArgs = {};
         Ping__ArgsCodec.encode(args, output);
         output.writeMessageEnd();
         return this.connection.send(writer.flush(), context).then((data: Buffer) => {
             const reader: thrift.TTransport = this.transport.receiver(data);
             const input: thrift.TProtocol = new this.protocol(reader);
             try {
                 const { fieldName: fieldName, messageType: messageType }: thrift.IThriftMessage = input.readMessageBegin();
                 if (fieldName === "ping") {
                     if (messageType === thrift.MessageType.EXCEPTION) {
                         const err: thrift.TApplicationException = thrift.TApplicationExceptionCodec.decode(input);
                         input.readMessageEnd();
                         return Promise.reject(err);
                     }
                     else {
-                        const result: IPing__Result = Ping__ResultCodec.decode(input);
+                        const result: Ping__Result = Ping__Result.read(input);
                         input.readMessageEnd();
                         if (result.success != null) {
                             return Promise.resolve(result.success);
                         }
                         else {
                             return Promise.reject(new thrift.TApplicationException(thrift.TApplicationExceptionType.UNKNOWN, "ping failed: unknown result"));
                         }
                     }
                 }
                 else {
                     return Promise.reject(new thrift.TApplicationException(thrift.TApplicationExceptionType.WRONG_METHOD_NAME, "Received a response to an unknown RPC function: " + fieldName));
                 }
             }
             catch (err) {
                 return Promise.reject(err);
             }
         });
     }

You'll notice that I also changed the type of result from IPing__Result to Ping__Result. This isn't important and could be reverted, but I think it's better this way. I like keeping types specific until the point where it's useful for them to be generic. I'd advocate for making the same change to the types of struct class members (would need to change these types, I think:

thrift-typescript/src/main/render/thrift-server/struct/class.ts

Line 293 in e8931ff

typeNodeForFieldType(field.fieldType, state),

and

thrift-typescript/src/main/render/thrift-server/struct/reader.ts

Line 51 in e8931ff

field.fieldType,

), though that isn't of immediately use to me personally and I don't intend to pursue it.

[markdoliner-doma]

I can clean this up a little (fix tests, update documentation as needed), but wanted to get feedback from Credit Karma folks to make sure I'm heading in the right direction before spending more time on it.

[hayes]

Unfortunately, as far as I can tell development stopped on this repo about a year ago. Up until that point they had been very responsive, and merged several of my PRs, then there stopped being any responses on issues and PRs around last September (I have a couple PRs that have been open since then). The company I was working for at the time ended up just creating an internal fork with the fixes we needed)

[ramakrishna-battala-ck]

Hello @mdoliner-st sorry for being unresponsive. Will actively look into the pending PR's and share some feedback soon.