fix(appium): fix incorrect handling of delete session with regard to plugin driver assignment

also take the opportunity to constantize a few more strings

packages/appium/lib/appium.js

@@ -3,7 +3,8 @@ import log from './logger';
 import { getBuildInfo, updateBuildInfo, APPIUM_VER } from './config';
 import { findMatchingDriver } from './drivers';
 import { BaseDriver, errors, isSessionCommand,
-         CREATE_SESSION_COMMAND } from '@appium/base-driver';
+         CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND, GET_STATUS_COMMAND
+} from '@appium/base-driver';
 import AsyncLock from 'async-lock';
 import { parseCapsForInnerDriver, pullSettings } from './utils';
 import { util } from '@appium/support';
@@ -443,9 +444,10 @@ class AppiumDriver extends BaseDriver {
     // The tricky part is that because we support command plugins, we need to wrap any of these
     // cases with plugin handling.
 
-    const isGetStatus = cmd === 'getStatus';
+    const isGetStatus = cmd === GET_STATUS_COMMAND;
+    const isDeleteSession = cmd === DELETE_SESSION_COMMAND;
     const isUmbrellaCmd = !isGetStatus && isAppiumDriverCommand(cmd);
-    const isSessionCmd = !isGetStatus && !isUmbrellaCmd;
+    const isSessionCmd = !isUmbrellaCmd || isDeleteSession;
 
     // if a plugin override proxying for this command and that is why we are here instead of just
     // letting the protocol proxy the command entirely, determine that, get the request object for
@@ -470,7 +472,9 @@ class AppiumDriver extends BaseDriver {
       }
       // now save the response protocol given that the session driver's protocol might differ
       protocol = dstSession.protocol;
-      driver = dstSession;
+      if (!isUmbrellaCmd) {
+        driver = dstSession;
+      }
     }
 
     // get any plugins which are registered as handling this command

packages/base-driver/lib/basedriver/driver.js

@@ -1,5 +1,5 @@
 import {
-  Protocol, errors, determineProtocol
+  Protocol, errors, determineProtocol, DELETE_SESSION_COMMAND,
 } from '../protocol';
 import { fs } from '@appium/support';
 import { PROTOCOLS, DEFAULT_BASE_PATH } from '../constants';
@@ -311,7 +311,7 @@ class BaseDriver extends Protocol {
       // If creating a session determine if W3C or MJSONWP protocol was requested and remember the choice
       this.protocol = determineProtocol(...args);
       this.logEvent(EVENT_SESSION_INIT);
-    } else if (cmd === 'deleteSession') {
+    } else if (cmd === DELETE_SESSION_COMMAND) {
       this.logEvent(EVENT_SESSION_QUIT_START);
     }
 
@@ -352,7 +352,7 @@ class BaseDriver extends Protocol {
     // automatic session deletion in this.onCommandTimeout. Of course we don't
     // want to trigger the timer when the user is shutting down the session
     // intentionally
-    if (this.isCommandsQueueEnabled && cmd !== 'deleteSession') {
+    if (this.isCommandsQueueEnabled && cmd !== DELETE_SESSION_COMMAND) {
       // resetting existing timeout
       this.startNewCommandTimeout();
     }
@@ -362,7 +362,7 @@ class BaseDriver extends Protocol {
     this._eventHistory.commands.push({cmd, startTime, endTime});
     if (cmd === 'createSession') {
       this.logEvent(EVENT_SESSION_START);
-    } else if (cmd === 'deleteSession') {
+    } else if (cmd === DELETE_SESSION_COMMAND) {
       this.logEvent(EVENT_SESSION_QUIT_DONE);
     }
 

packages/base-driver/lib/index.js

@@ -21,7 +21,8 @@ const {
   Protocol, routeConfiguringFunction, errors, isErrorType,
   errorFromMJSONWPStatusCode, errorFromW3CJsonCode, ALL_COMMANDS, METHOD_MAP,
   routeToCommandName, NO_SESSION_ID_COMMANDS, isSessionCommand,
-  normalizeBasePath, determineProtocol, CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND,
+  normalizeBasePath, determineProtocol, CREATE_SESSION_COMMAND,
+  DELETE_SESSION_COMMAND, GET_STATUS_COMMAND,
 } = protocol;
 
 export {
@@ -29,7 +30,8 @@ export {
   errorFromMJSONWPStatusCode, errorFromW3CJsonCode, determineProtocol,
   errorFromMJSONWPStatusCode as errorFromCode, ALL_COMMANDS, METHOD_MAP,
   routeToCommandName, NO_SESSION_ID_COMMANDS, isSessionCommand,
-  DEFAULT_BASE_PATH, normalizeBasePath, CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND
+  DEFAULT_BASE_PATH, normalizeBasePath, CREATE_SESSION_COMMAND,
+  DELETE_SESSION_COMMAND, GET_STATUS_COMMAND,
 };
 
 // Express exports

packages/base-driver/lib/protocol/protocol.js

@@ -16,6 +16,7 @@ import SESSIONS_CACHE from './sessions-cache';
 
 const CREATE_SESSION_COMMAND = 'createSession';
 const DELETE_SESSION_COMMAND = 'deleteSession';
+const GET_STATUS_COMMAND = 'getStatus';
 
 class Protocol {}
 
@@ -416,7 +417,7 @@ function driverShouldDoJwpProxy (driver, req, command) {
 
   // we should never proxy deleteSession because we need to give the containing
   // driver an opportunity to clean itself up
-  if (command === 'deleteSession') {
+  if (command === DELETE_SESSION_COMMAND) {
     return false;
   }
 
@@ -451,5 +452,6 @@ async function doJwpProxy (driver, req, res) {
 
 export {
   Protocol, routeConfiguringFunction, isSessionCommand,
-  driverShouldDoJwpProxy, determineProtocol, CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND,
+  driverShouldDoJwpProxy, determineProtocol, CREATE_SESSION_COMMAND,
+  DELETE_SESSION_COMMAND, GET_STATUS_COMMAND,
 };