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

Node.js Thrift connection libraries throw Exceptions into event emitter

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 0.9.2
    • Fix Version/s: 0.9.3
    • Component/s: Node.js - Library
    • Labels:
      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

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

              Dates

              • Created:
                Updated:
                Resolved: