Skip to content

Commit

Permalink
Merge pull request #347 from forcedotcom/sm/W-8206747
Browse files Browse the repository at this point in the history
Sm/w 8206747
  • Loading branch information
peternhale authored Jan 13, 2021
2 parents c13fe4f + 794c99b commit 10fc422
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 17 deletions.
43 changes: 40 additions & 3 deletions src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { maxBy, merge } from '@salesforce/kit';
import { URL } from 'url';
import { Duration, maxBy, merge } from '@salesforce/kit';
import { asString, ensure, getNumber, isString, JsonCollection, JsonMap, Optional } from '@salesforce/ts-types';
import {
Connection as JSForceConnection,
Expand All @@ -17,6 +17,8 @@ import {
Tooling as JSForceTooling,
} from 'jsforce';
import { AuthFields, AuthInfo } from './authInfo';
import { MyDomainResolver } from './status/myDomainResolver';

import { ConfigAggregator } from './config/configAggregator';
import { Logger } from './logger';
import { SfdxError } from './sfdxError';
Expand All @@ -36,6 +38,8 @@ export const SFDX_HTTP_HEADERS = {
'user-agent': clientId,
};

export const DNS_ERROR_NAME = 'Domain Not Found';

// This interface is so we can add the autoFetchQuery method to both the Connection
// and Tooling classes and get nice typing info for it within editors. JSForce is
// unlikely to accept a PR for this method, but that would be another approach.
Expand Down Expand Up @@ -119,6 +123,8 @@ export class Connection extends JSForceConnection {

const conn = new this(options);
await conn.init();
// verifies that subsequent requests to org will not hit DNS errors

if (!versionFromConfig) {
await conn.useLatestApiVersion();
}
Expand Down Expand Up @@ -178,10 +184,12 @@ export class Connection extends JSForceConnection {
* Retrieves the highest api version that is supported by the target server instance.
*/
public async retrieveMaxApiVersion(): Promise<string> {
await this.isResolvable();
type Versioned = { version: string };
const versions = (await this.request(`${this.instanceUrl}/services/data`)) as Versioned[];
this.logger.debug(`response for org versions: ${versions}`);
this.logger.debug(`response for org versions: ${versions.map((item) => item.version).join(',')}`);
const max = ensure(maxBy(versions, (version: Versioned) => version.version));

return max.version;
}
/**
Expand All @@ -191,11 +199,40 @@ export class Connection extends JSForceConnection {
try {
this.setApiVersion(await this.retrieveMaxApiVersion());
} catch (err) {
if (err.name === DNS_ERROR_NAME) {
throw err; // throws on DNS connection errors
}
// Don't fail if we can't use the latest, just use the default
this.logger.warn('Failed to set the latest API version:', err);
}
}

/**
* Verify that instance has a reachable DNS entry, otherwise will throw error
*/
public async isResolvable(): Promise<boolean> {
if (!this.options.connectionOptions?.instanceUrl) {
throw new SfdxError('Connection has no instanceUrl', 'NoInstanceUrl', [
'Make sure the instanceUrl is set in your command or config',
]);
}
const resolver = await MyDomainResolver.create({
url: new URL(this.options.connectionOptions.instanceUrl),
timeout: Duration.seconds(10),
frequency: Duration.seconds(10),
});
try {
await resolver.resolve();
return true;
} catch (e) {
throw new SfdxError('The org cannot be found', DNS_ERROR_NAME, [
'Verify that the org still exists',
'If your org is newly created, wait a minute and run your command again',
"If you deployed or updated the org's My Domain, logout from the CLI and authenticate again",
]);
}
}

/**
* Get the API version used for all connection requests.
*/
Expand Down
59 changes: 45 additions & 14 deletions test/unit/connectionTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import { get, JsonMap } from '@salesforce/ts-types';
import { assert, expect } from 'chai';
import * as jsforce from 'jsforce';
import { AuthInfo } from '../../src/authInfo';
import { MyDomainResolver } from '../../src/status/myDomainResolver';
import { ConfigAggregator, ConfigInfo } from '../../src/config/configAggregator';
import { Connection, SFDX_HTTP_HEADERS, SingleRecordQueryErrors } from '../../src/connection';
import { testSetup } from '../../src/testSetup';
import { Connection, SFDX_HTTP_HEADERS, DNS_ERROR_NAME, SingleRecordQueryErrors } from '../../src/connection';
import { testSetup, shouldThrow } from '../../src/testSetup';

// Setup the test environment.
const $$ = testSetup();
const TEST_IP = '1.1.1.1';

describe('Connection', () => {
const testConnectionOptions = { loginUrl: 'connectionTest/loginUrl' };
Expand All @@ -27,13 +29,34 @@ describe('Connection', () => {
getConnectionOptions: () => testConnectionOptions,
};

const testAuthInfoWithDomain = {
...testAuthInfo,
getConnectionOptions: () => ({
...testConnectionOptions,
instanceUrl: 'https://connectionTest/instanceUrl',
}),
};

beforeEach(() => {
$$.SANDBOXES.CONNECTION.restore();
$$.SANDBOX.stub(process, 'emitWarning');
$$.SANDBOX.stub(MyDomainResolver.prototype, 'resolve').resolves(TEST_IP);

initializeStub = $$.SANDBOX.stub(jsforce.Connection.prototype, 'initialize').returns();
requestMock = $$.SANDBOX.stub(jsforce.Connection.prototype, 'request')
.onFirstCall()
.returns(Promise.resolve([{ version: '42.0' }]));
.resolves([{ version: '42.0' }]);
});

it('create() should throw on DNS errors', async () => {
$$.SANDBOX.restore();
$$.SANDBOX.stub(MyDomainResolver.prototype, 'resolve').rejects({ name: DNS_ERROR_NAME });

try {
await shouldThrow(Connection.create({ authInfo: testAuthInfoWithDomain as AuthInfo }));
} catch (e) {
expect(e).to.have.property('name', DNS_ERROR_NAME);
}
});

it('create() should create a connection using AuthInfo and SFDX options', async () => {
Expand Down Expand Up @@ -73,8 +96,9 @@ describe('Connection', () => {
headers: SFDX_HTTP_HEADERS,
};

const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });

const conn = await Connection.create({
authInfo: testAuthInfoWithDomain as AuthInfo,
});
// Test passing a string to conn.request()
const response1 = await conn.request(testUrl);
expect(requestMock.called).to.be.true;
Expand All @@ -88,7 +112,9 @@ describe('Connection', () => {
requestMock.onSecondCall().returns(Promise.resolve(testResponse));
const testUrl = 'connectionTest/request/url/describe';

const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const conn = await Connection.create({
authInfo: testAuthInfoWithDomain as AuthInfo,
});

// Test passing a RequestInfo object and options to conn.request()
const requestInfo = { method: 'POST', url: testUrl };
Expand All @@ -107,7 +133,9 @@ describe('Connection', () => {
const testResponse = { success: true };
requestMock.onSecondCall().returns(Promise.resolve(testResponse));

const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const conn = await Connection.create({
authInfo: testAuthInfoWithDomain as AuthInfo,
});

const testUrl = '/services/data/v42.0/tooling/sobjects';
const requestInfo = { method: 'GET', url: testUrl };
Expand Down Expand Up @@ -197,7 +225,9 @@ describe('Connection', () => {
it('autoFetch() should reject the promise upon query error', async () => {
const errorMsg = 'QueryFailed';
requestMock.onSecondCall().throws(new Error(errorMsg));
const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const conn = await Connection.create({
authInfo: testAuthInfoWithDomain as AuthInfo,
});

try {
await conn.autoFetchQuery('TEST_SOQL');
Expand All @@ -212,10 +242,11 @@ describe('Connection', () => {
id: '123',
name: 'testName',
};
requestMock.returns(Promise.resolve({ totalSize: 1, records: [mockSingleRecord] }));
const soql = 'TEST_SOQL';
requestMock.onSecondCall().resolves({ totalSize: 1, records: [mockSingleRecord] });

const conn = await Connection.create({ authInfo: testAuthInfoWithDomain as AuthInfo });

const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const queryResult = await conn.singleRecordQuery(soql);
expect(queryResult).to.deep.equal({
...mockSingleRecord,
Expand All @@ -224,7 +255,7 @@ describe('Connection', () => {

it('singleRecordQuery throws on no-records', async () => {
requestMock.returns(Promise.resolve({ totalSize: 0, records: [] }));
const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const conn = await Connection.create({ authInfo: testAuthInfoWithDomain as AuthInfo });

try {
await conn.singleRecordQuery('TEST_SOQL');
Expand All @@ -236,7 +267,7 @@ describe('Connection', () => {

it('singleRecordQuery throws on multiple records', async () => {
requestMock.returns(Promise.resolve({ totalSize: 2, records: [{ id: 1 }, { id: 2 }] }));
const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const conn = await Connection.create({ authInfo: testAuthInfoWithDomain as AuthInfo });

try {
await conn.singleRecordQuery('TEST_SOQL');
Expand All @@ -248,7 +279,7 @@ describe('Connection', () => {

it('singleRecordQuery throws on multiple records with options', async () => {
requestMock.returns(Promise.resolve({ totalSize: 2, records: [{ id: 1 }, { id: 2 }] }));
const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const conn = await Connection.create({ authInfo: testAuthInfoWithDomain as AuthInfo });

try {
await conn.singleRecordQuery('TEST_SOQL', { returnChoicesOnMultiple: true, choiceField: 'id' });
Expand All @@ -267,7 +298,7 @@ describe('Connection', () => {
requestMock.returns(Promise.resolve({ totalSize: 1, records: [mockSingleRecord] }));
const soql = 'TEST_SOQL';

const conn = await Connection.create({ authInfo: testAuthInfo as AuthInfo });
const conn = await Connection.create({ authInfo: testAuthInfoWithDomain as AuthInfo });
const toolingQuerySpy = $$.SANDBOX.spy(conn.tooling, 'query');
const queryResults = await conn.singleRecordQuery(soql, { tooling: true });
expect(queryResults).to.deep.equal({
Expand Down
6 changes: 6 additions & 0 deletions test/unit/orgTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { Global } from '../../src/global';
import { Org } from '../../src/org';
import { MockTestOrgData, testSetup } from '../../src/testSetup';
import { fs } from '../../src/util/fs';
import { MyDomainResolver } from '../../src/status/myDomainResolver';

const $$ = testSetup();

Expand All @@ -38,6 +39,8 @@ describe('Org Tests', () => {
beforeEach(async () => {
testData = new MockTestOrgData();
$$.configStubs.AuthInfoConfig = { contents: await testData.getConfig() };
$$.SANDBOX.stub(MyDomainResolver.prototype, 'resolve').resolves('1.1.1.1');

stubMethod($$.SANDBOX, Connection.prototype, 'useLatestApiVersion').returns(Promise.resolve());
});

Expand Down Expand Up @@ -122,6 +125,9 @@ describe('Org Tests', () => {
const org: Org = await Org.create({
connection: await Connection.create({
authInfo: await AuthInfo.create({ username: testData.username }),
connectionOptions: {
instanceUrl: 'https://orgTest.instanceUrl',
},
}),
});
const apiVersion = await org.retrieveMaxApiVersion();
Expand Down

0 comments on commit 10fc422

Please sign in to comment.