[THRIFT-5674] Server implementation exceptions are not sent to client in ES6 promise-style invocation

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.17.0
• Fix Version/s: 0.19.0
• Component/s: JavaScript - Compiler

Environment

Discovered on Node.js v16.13.2, Thrift 0.17.0 compiler, 0.16.0 library.

Reproduced on Node.js v10.19.0, Thrift 0.17.0 compiler, 0.17.0 library (just what I happened to have available for trying out the tutorial code).

• Assignee: Chandler May
• Reporter: Chandler May
• Created: 18/Dec/22 18:57
• Updated: 02/Sep/23 11:53
• Resolved: 01/Mar/23 23:12

Description

When a server implementation throws a user-defined exception, the server crashes and the exception is not sent to the client.  I believe this issue only happens in the Promise-style invocation of code generated for ES6.  Specifically, I think it arises from this line:

https://github.com/apache/thrift/blob/0.17.0/compiler/cpp/src/thrift/generate/t_js_generator.cc#L1484

Generated code looks like the following (generated from the Calculator.calculate example):

Promise.resolve(this._handler.calculate.bind(this._handler)(
  args.logid,
  args.w
)).then(
  ...
).catch(err => {
  if (err instanceof ttypes.InvalidOperation) {
    ...
  }
);

The implementation-generated exception is handled by the promise chain.  The problem is that the implementation (handler) method is called before Promise.resolve, so any exception it throws is not handled by the promise chain.  So, when running¹ NodeServerPromise.js and NodeClientPromise.js from the tutorial, the server crashes when InvalidOperation is thrown and the client does not receive InvalidOperation (or anything further).

I think the minimal fix would be to generate something like the following instead:

new Promise((resolve) => resolve(this._handler.calculate.bind(this._handler)(
  args.logid,
  args.w
))).then(
  ...
).catch(err => {
  if (err instanceof ttypes.InvalidOperation) {
    ...
  }
);

When making this change on the generated tutorial code and re-running¹ the server and client, the server stays up and both the server and client log five lines, as expected.

[1] There also seems to be a couple of errors in NodeClientPromise.js in the tutorial code.  I had to change "fail" to "catch" and "fin" to "finally" to get it to work.

Issue Links

• links to: GitHub Pull Request #2739