Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-2932

Node.js Thrift connection libraries throw Exceptions into event emitter

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • 0.9.2
    • 0.9.3
    • Node.js - Library
    • None

    Description

      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:

          catch (e) {
            if (e instanceof ttransport.InputBufferUnderrunError) {
              transport_with_data.rollbackPosition();
            }
            else {
              throw e;
            }
          }
      

      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.

      Attachments

        Activity

          People

            codesf Randy Abernethy
            sh1mmer Tom Croucher
            Votes:
            3 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: