[ https://issues.apache.org/jira/browse/THRIFT-3916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16397108#comment-16397108 ]
ASF GitHub Bot commented on THRIFT-3916: ---------------------------------------- Github user bananer commented on the issue: https://github.com/apache/thrift/pull/1075 @jeking3 This is a well-intended change but I think it is not implemented correctly and will break error handling for users. In some places in the compiler the input to `render_recv_throw` already is a proper `Error` (the JS exception class), e.g. https://github.com/apache/thrift/blob/2b09dfed9c6b858571e7d8829a2b4a4bcda18d6a/compiler/cpp/src/thrift/generate/t_js_generator.cc#L1648-L1652 This is not the case for other places: https://github.com/apache/thrift/blob/2b09dfed9c6b858571e7d8829a2b4a4bcda18d6a/compiler/cpp/src/thrift/generate/t_js_generator.cc#L1673 The proper way of doing this would be to inspect all usages of `render_recv_throw` to see if the argument needs to be wrapped in `new Error(…)`. Still then, this could break error handling that relies on these cases being strings. > Errors thrown from JavaScript client is strings and not errors > -------------------------------------------------------------- > > Key: THRIFT-3916 > URL: https://issues.apache.org/jira/browse/THRIFT-3916 > Project: Thrift > Issue Type: Bug > Components: JavaScript - Compiler > Reporter: Simen Bekkhus > Priority: Minor > Labels: javascript > > In JavaScript, you can {{throw}} any object, including strings, but to get > stack traces one should throw {{Errors}}. > The generated JS code throws the error string directly, instead of wrapping > it in {{new Error}}. > Node core _only_ rejects with/throws Errors, never strings, and Thrift should > follow the same standard. > https://nodejs.org/api/errors.html#errors_class_error > {quote} > All errors generated by Node.js, including all System and JavaScript errors, > will either be instances of, or inherit from, the Error class. > {quote} > PR for the change [here|https://github.com/apache/thrift/pull/1075]. -- This message was sent by Atlassian JIRA (v7.6.3#76005)