[ 
https://issues.apache.org/jira/browse/THRIFT-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14294207#comment-14294207
 ] 

Randy Abernethy commented on THRIFT-2932:
-----------------------------------------

Both of these libs predate my involvement in Thrift but when first exposed to 
them a couple years ago they were primordial and supporting only very limited 
functionality. The browser lib only did JSON/XHR/HTTP and Node only did 
Binary/Socket so they could not talk to each other (browser was/is tested 
against Java). No npm or bower support at that time either. 

The libs are now out on npm and bower (which is a mess due to the whole repo 
download thing). Compact and JSON protocols were added to Node. Many things 
that didn't work at all were fixed and lots of tests were added. Other than the 
tests (which were needed just to get things working more broadly) all of this 
work has been feature oriented and incremental. People tend to provide patches 
to fix or add things they need, not clean up stuff that is a mess (partly 
because it might break existing code, for example changing the call back 
interface in the browser to add error objects). So we have some baggage...

That said, you are not alone. Many would like to see the kind of improvements 
mentioned here and in other tickets. There's still time to make a big push 
before Thrift v1.0. Patches welcome!

> Node.js Thrift connection libraries throw Exceptions into event emitter
> -----------------------------------------------------------------------
>
>                 Key: THRIFT-2932
>                 URL: https://issues.apache.org/jira/browse/THRIFT-2932
>             Project: Thrift
>          Issue Type: Bug
>          Components: Node.js - Library
>    Affects Versions: 0.9.2
>            Reporter: Tom Croucher
>            Priority: Critical
>
> I've been investigating using the Node.js client in a project however it 
> seems like there are instances which don't follow Node.js best practices. 
> In particular http_connection.js and connection.js throw errors during 
> callbacks. This is considered an anti-pattern in Node because it both removes 
> the Exception from the context of the callback making it hard to associate 
> with a request as well as throwing it in the context of the EventEmitter code 
> which can cause inconsistencies in the Node process.
> This means under some error conditions an uncaught exception would be thrown 
> or at least an 'error' event on the singleton client (again removing it from 
> the request context).
> Both transport receivers share the same copy-pasta code which contains:
> {code:javascript}
>     catch (e) {
>       if (e instanceof ttransport.InputBufferUnderrunError) {
>         transport_with_data.rollbackPosition();
>       }
>       else {
>         throw e;
>       }
>     }
> {code}
> I'm working on a patch, but I'm curious about some of the history of the 
> code. In particular the exception based loop flow control and the using the 
> seqid to track the callback which makes it hard to properly associate it with 
> exception handling.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to