[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication

[jorgebay]

Github user jorgebay commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/889#discussion_r201295616
  
    --- Diff: 
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
 ---
    @@ -151,7 +156,16 @@ class DriverRemoteConnection extends RemoteConnection {
           return;
         }
     
    -    if (response.status.code >= 400) {
    +    if (response.status.code === 
responseStatusCode.authenticationChallenge && this._authenticator) {
    +      this._authenticator.evaluateChallenge(response).then(res => {
    --- End diff --
    
    To avoid leaking those response handlers, you should call `_clearHandler()`:
    
    ```javascript
    // Similar to how its being done below, maybe we can move that to a 
separate method
    this._clearHandler(response.requestId);
    handler.callback(null, { traversers: [] });
    ```
    
    So the final solution should be something like:
    ```javascript
    this._authenticator.evaluateChallenge(response)
      .then(() => this.submit(null, 'authentication', res))
      .then(() => this._clearHandlerAndInvokeCallback(response.requestId, 
handler, []))
      .catch(handler.callback);
    return;
    ```


---