Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

fix: Patch downstream responses for unknown upstream protocol #353

Merged
merged 3 commits into from
Sep 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/jsonwp-proxy/protocol-converter.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import _ from 'lodash';
import { logger, util } from 'appium-support';
import { duplicateKeys } from '../basedriver/helpers';
import { PROTOCOLS, MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY } from '../protocol/protocol';
import { errors } from '../protocol/errors';

const log = logger.getLogger('Protocol Converter');

Expand Down Expand Up @@ -187,6 +188,18 @@ class ProtocolConverter {
: await this.proxyFunc(url, method, body);
}

async proxyGetStatus (url, method, body) {
const [res, resBodyObj] = await this.proxyFunc(url, method, body);
if (!_.isPlainObject(resBodyObj)) {
return [res, resBodyObj];
}
const patchedBody = _.clone(resBodyObj);
if (!_.has(resBodyObj, 'status')) {
resBodyObj.status = res.statusCode === 200 ? 0 : errors.UnknownError.code();
}
return [res, patchedBody];
}

/**
* Handle "crossing" endpoints for the case
* when upstream and downstream drivers operate different protocols
Expand All @@ -199,6 +212,11 @@ class ProtocolConverter {
*/
async convertAndProxy (commandName, url, method, body) {
if (!this.downstreamProtocol) {
if (commandName === 'getStatus') {
// Patch getStatus call for GENERIC protocol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀
ah...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are existing tests, I would say they do cover such changes

// to preserve the backward compatibility
return await this.proxyGetStatus(url, method, body);
}
// There is no point to convert anything if we do not know
// for which protocol the conversion should be done
return await this.proxyFunc(url, method, body);
Expand Down
34 changes: 7 additions & 27 deletions lib/jsonwp-proxy/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,28 +118,6 @@ class JWProxy {
return proxyBase + remainingUrl;
}

syncDownstreamProtocol (resBodyObj, isSessionCreationRequest) {
if (!this.downstreamProtocol) {
this.downstreamProtocol = this.getProtocolFromResBody(resBodyObj);
log.debug(`Determined the downstream protocol as '${this.downstreamProtocol}'`);
} else if (isSessionCreationRequest) {
// It might be that we proxy API calls to the downstream driver
// without creating a session first
// and it responds using the default proto,
// but then after createSession request is sent the internal proto is changed
// to the other one based on the actually provided caps
const previousValue = this.downstreamProtocol;
this.downstreamProtocol = this.getProtocolFromResBody(resBodyObj);
if (previousValue && previousValue !== this.downstreamProtocol) {
log.debug(`Updated the downstream protocol to '${this.downstreamProtocol}' ` +
`as per session creation request`);
} else {
log.debug(`Determined the downstream protocol as '${this.downstreamProtocol}' ` +
`per session creation request`);
}
}
}

async proxy (url, method, body = null) {
method = method.toUpperCase();
const newUrl = this.getUrlForProxy(url);
Expand Down Expand Up @@ -201,12 +179,14 @@ class JWProxy {
log.debug(`Got response with status ${res.statusCode}: ${truncateBody(res.body)}`);
isResponseLogged = true;
const isSessionCreationRequest = /\/session$/.test(url) && method === 'POST';
if (isSessionCreationRequest && res.statusCode === 200) {
this.sessionId = resBodyObj.sessionId || (resBodyObj.value || {}).sessionId;
if (isSessionCreationRequest) {
if (res.statusCode === 200) {
this.sessionId = resBodyObj.sessionId || (resBodyObj.value || {}).sessionId;
}
this.downstreamProtocol = this.getProtocolFromResBody(resBodyObj);
log.info(`Determined the downstream protocol as '${this.downstreamProtocol}'`);
}
this.syncDownstreamProtocol(resBodyObj, isSessionCreationRequest);
if (res.statusCode < 400 && this.downstreamProtocol === MJSONWP &&
_.has(resBodyObj, 'status') && parseInt(resBodyObj.status, 10) !== 0) {
if (res.statusCode < 400 && _.has(resBodyObj, 'status') && parseInt(resBodyObj.status, 10) !== 0) {
// Some servers, like chromedriver may return response code 200 for non-zero JSONWP statuses
throwProxyError(resBodyObj);
}
Expand Down