From c4347f34bdfbf6bb1092c73f26ab4f6c18fd887a Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Thu, 9 Apr 2026 01:35:12 +0200 Subject: [PATCH 1/2] Add test for ES6 generated exception constructor Client: js Add unit tests verifying that ES6-generated exception classes pass the exception name string to super() instead of the args object. This is a regression test for the bug fixed in PR #3372, where super(args) caused TBinaryProtocol.writeStringOrBinary to throw. Co-Authored-By: Claude Sonnet 4.6 --- lib/nodejs/test/generated-exceptions.test.js | 87 ++++++++++++++++++++ lib/nodejs/test/testAll.sh | 1 + 2 files changed, 88 insertions(+) create mode 100644 lib/nodejs/test/generated-exceptions.test.js diff --git a/lib/nodejs/test/generated-exceptions.test.js b/lib/nodejs/test/generated-exceptions.test.js new file mode 100644 index 0000000000..2104916872 --- /dev/null +++ b/lib/nodejs/test/generated-exceptions.test.js @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +"use strict"; + +const test = require("tape"); +const thrift = require("thrift"); + +// ES5 generated types (pre-ES6 path) +const ttypesEs5 = require("./gen-nodejs/ThriftTest_types"); +// ES6 generated types +const ttypesEs6 = require("./gen-nodejs-es6/ThriftTest_types"); + +function serializeBinary(data) { + let buff; + const transport = new thrift.TBufferedTransport(null, function (msg) { + buff = msg; + }); + const prot = new thrift.TBinaryProtocol(transport); + data[Symbol.for("write")](prot); + prot.flush(); + return buff; +} + +// Test that ES6 generated exception constructor passes the exception name +// (not the args object) to super(), matching the ES5 behavior. +// Regression test for: https://github.com/apache/thrift/pull/3372 + +test("ES6 generated exception - constructor sets name and message correctly", function t(assert) { + const e = new ttypesEs6.Xception({ errorCode: 1001, message: "test error" }); + assert.ok(e instanceof thrift.Thrift.TException, "is instanceof TException"); + assert.ok(e instanceof Error, "is instanceof Error"); + assert.equal(e.name, "Xception", "name is set to exception class name"); + assert.equal(e.errorCode, 1001, "custom field errorCode is set"); + assert.equal(typeof e.stack, "string", "has stack trace"); + assert.end(); +}); + +test("ES6 generated exception - super() receives string, not args object", function t(assert) { + const e = new ttypesEs6.Xception({ errorCode: 1001, message: "test error" }); + // The bug was that super(args) passed the args object to TException, + // which would cause message to be "[object Object]" + assert.notEqual(e.message, "[object Object]", + "message is not '[object Object]' (would indicate args object was passed to super)"); + assert.end(); +}); + +test("ES6 generated exception - serialization does not throw", function t(assert) { + const e = new ttypesEs6.Xception({ errorCode: 1001, message: "test error" }); + assert.doesNotThrow(function () { + serializeBinary(e); + }, "serializing an ES6 exception should not throw"); + assert.end(); +}); + +test("ES5 generated exception - constructor sets name and message correctly", function t(assert) { + const e = new ttypesEs5.Xception({ errorCode: 1001, message: "test error" }); + assert.ok(e instanceof thrift.Thrift.TException, "is instanceof TException"); + assert.ok(e instanceof Error, "is instanceof Error"); + assert.equal(e.name, "Xception", "name is set to exception class name"); + assert.equal(e.errorCode, 1001, "custom field errorCode is set"); + assert.end(); +}); + +test("ES5 and ES6 generated exceptions have consistent behavior", function t(assert) { + const es5 = new ttypesEs5.Xception({ errorCode: 1001, message: "test error" }); + const es6 = new ttypesEs6.Xception({ errorCode: 1001, message: "test error" }); + assert.equal(es5.name, es6.name, "name matches between ES5 and ES6"); + assert.equal(es5.errorCode, es6.errorCode, "errorCode matches between ES5 and ES6"); + assert.end(); +}); diff --git a/lib/nodejs/test/testAll.sh b/lib/nodejs/test/testAll.sh index d37f9a31a7..182e260231 100755 --- a/lib/nodejs/test/testAll.sh +++ b/lib/nodejs/test/testAll.sh @@ -133,6 +133,7 @@ node ${DIR}/binary.test.js || TESTOK=1 node ${DIR}/header.test.js || TESTOK=1 node ${DIR}/int64.test.js || TESTOK=1 node ${DIR}/deep-constructor.test.js || TESTOK=1 +node ${DIR}/generated-exceptions.test.js || TESTOK=1 node ${DIR}/include.test.mjs || TESTOK=1 node ${DIR}/thrift_4987_xhr_protocol.test.mjs || TESTOK=1 From aa904d08c8e70d3838e0ae373cf27e237fd861ab Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Thu, 9 Apr 2026 01:50:39 +0200 Subject: [PATCH 2/2] Fix prettier formatting in generated-exceptions test Co-Authored-By: Claude Sonnet 4.6 --- lib/nodejs/test/generated-exceptions.test.js | 23 +++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/nodejs/test/generated-exceptions.test.js b/lib/nodejs/test/generated-exceptions.test.js index 2104916872..9a3c54fb80 100644 --- a/lib/nodejs/test/generated-exceptions.test.js +++ b/lib/nodejs/test/generated-exceptions.test.js @@ -56,8 +56,11 @@ test("ES6 generated exception - super() receives string, not args object", funct const e = new ttypesEs6.Xception({ errorCode: 1001, message: "test error" }); // The bug was that super(args) passed the args object to TException, // which would cause message to be "[object Object]" - assert.notEqual(e.message, "[object Object]", - "message is not '[object Object]' (would indicate args object was passed to super)"); + assert.notEqual( + e.message, + "[object Object]", + "message is not '[object Object]' (would indicate args object was passed to super)", + ); assert.end(); }); @@ -79,9 +82,19 @@ test("ES5 generated exception - constructor sets name and message correctly", fu }); test("ES5 and ES6 generated exceptions have consistent behavior", function t(assert) { - const es5 = new ttypesEs5.Xception({ errorCode: 1001, message: "test error" }); - const es6 = new ttypesEs6.Xception({ errorCode: 1001, message: "test error" }); + const es5 = new ttypesEs5.Xception({ + errorCode: 1001, + message: "test error", + }); + const es6 = new ttypesEs6.Xception({ + errorCode: 1001, + message: "test error", + }); assert.equal(es5.name, es6.name, "name matches between ES5 and ES6"); - assert.equal(es5.errorCode, es6.errorCode, "errorCode matches between ES5 and ES6"); + assert.equal( + es5.errorCode, + es6.errorCode, + "errorCode matches between ES5 and ES6", + ); assert.end(); });