Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sm/w 8206747 #347

Merged
merged 13 commits into from
Jan 13, 2021
40 changes: 32 additions & 8 deletions src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 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 { URL } from 'url';
import { maxBy, merge } from '@salesforce/kit';
import { asString, ensure, getNumber, isString, JsonCollection, JsonMap, Optional } from '@salesforce/ts-types';
import {
Expand All @@ -16,7 +16,10 @@ import {
RequestInfo,
Tooling as JSForceTooling,
} from 'jsforce';
import { Duration } from '@salesforce/kit';
import { AuthFields, AuthInfo } from './authInfo';
import { MyDomainResolver } from './status/myDomainResolver';

mshanemc marked this conversation as resolved.
Show resolved Hide resolved
import { ConfigAggregator } from './config/configAggregator';
import { Logger } from './logger';
import { SfdxError } from './sfdxError';
Expand Down Expand Up @@ -119,8 +122,12 @@ export class Connection extends JSForceConnection {

const conn = new this(options);
await conn.init();
if (!versionFromConfig) {
await conn.useLatestApiVersion();
if (!versionFromConfig && conn.options?.connectionOptions?.instanceUrl) {
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
try {
await conn.useLatestApiVersion();
} catch (e) {
conn.logger.error(`Cannot connect to the org at ${conn.options?.connectionOptions?.instanceUrl}`);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

useLatestApiVersion also has a try/catch and it does not rethrow, so this try/catch seems unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did some updates after Steve's feedback and a lot more e2e testing. That try/catch in useLatestApiVersion is now more error-specific for DNS problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what other scenario would prevent us from being able to determine the version, though.

}
return conn;
}
Expand Down Expand Up @@ -178,11 +185,28 @@ export class Connection extends JSForceConnection {
* Retrieves the highest api version that is supported by the target server instance.
*/
public async retrieveMaxApiVersion(): Promise<string> {
type Versioned = { version: string };
const versions = (await this.request(`${this.instanceUrl}/services/data`)) as Versioned[];
this.logger.debug(`response for org versions: ${versions}`);
const max = ensure(maxBy(versions, (version: Versioned) => version.version));
return max.version;
// errors go to stdout from within jsforce (maybe even further down the stack) if org's url doesn't exist. (not a 404 from the domain but a DNS ENOTFOUND)
// We need to verify that the org can be connected to BEFORE asking about it's api version. This will throw on DNS errors
if (!this.options.connectionOptions?.instanceUrl) {
throw new Error('Connection has no instanceUrl');
}
const resolver = await MyDomainResolver.create({
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
url: new URL(this.options.connectionOptions.instanceUrl),
timeout: Duration.minutes(0),
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
});
const ipAddress = await resolver.resolve();

if (ipAddress) {
type Versioned = { version: string };
const versions = (await this.request(
`${this.options.connectionOptions.instanceUrl}/services/data`
)) as Versioned[];
this.logger.debug(`response for org versions: ${versions}`);
const max = ensure(maxBy(versions, (version: Versioned) => version.version));
return max.version;
}

throw new Error();
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
}
/**
* Use the latest API version available on `this.instanceUrl`.
Expand Down
30 changes: 25 additions & 5 deletions test/unit/connectionTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ 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 } from '../../src/connection';
import { testSetup } 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,9 +30,19 @@ describe('Connection', () => {
getConnectionOptions: () => testConnectionOptions,
};

const testAuthInfoWithDomain = {
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
...testAuthInfo,
getConnectionOptions: () => ({
...testConnectionOptions,
instanceUrl: 'https://connectionTest/instanceUrl',
}),
};

beforeEach(() => {
$$.SANDBOXES.CONNECTION.restore();
$$.SANDBOX.stub(process, 'emitWarning');
$$.SANDBOX.stub(MyDomainResolver.prototype, 'resolve').resolves(TEST_IP);
mshanemc marked this conversation as resolved.
Show resolved Hide resolved

initializeStub = $$.SANDBOX.stub(jsforce.Connection.prototype, 'initialize').returns();
requestMock = $$.SANDBOX.stub(jsforce.Connection.prototype, 'request')
.onFirstCall()
Expand Down Expand Up @@ -73,8 +86,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 +102,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 +123,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 +215,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({
peternhale marked this conversation as resolved.
Show resolved Hide resolved
authInfo: testAuthInfoWithDomain as AuthInfo,
});

try {
await conn.autoFetchQuery('TEST_SOQL');
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