Skip to content

Commit 37f7ede

Browse files
authored
fix(endpoints): guard against remote call hanging #3341
Problem: On startup, Toolkit tries to fetch https://idetoolkits.amazonwebservices.com/endpoints.json in EndpointsProvider#load. If that URL is is blocked by firewall, the toolkit is blocked and unusuable. #3338 Solution: First load local endpoints.json then dynamically update with the result of remote call once it receives a response.
1 parent 61bbc24 commit 37f7ede

File tree

7 files changed

+96
-141
lines changed

7 files changed

+96
-141
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "endpoints: local file never loads if remote call hangs"
4+
}

src/extension.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ import {
3232
} from './shared/extensionUtilities'
3333
import { getLogger, Logger } from './shared/logger/logger'
3434
import { activate as activateLogger } from './shared/logger/activation'
35-
import { RegionProvider } from './shared/regions/regionProvider'
36-
import { EndpointsProvider } from './shared/regions/endpointsProvider'
35+
import { getEndpointsFromFetcher, RegionProvider } from './shared/regions/regionProvider'
3736
import { FileResourceFetcher } from './shared/resourcefetcher/fileResourceFetcher'
3837
import { HttpResourceFetcher } from './shared/resourcefetcher/httpResourceFetcher'
3938
import { activate as activateEcr } from './ecr/activation'
@@ -299,13 +298,14 @@ function initializeCredentialsProviderManager() {
299298
manager.addProviders(new Ec2CredentialsProvider(), new EcsCredentialsProvider(), new EnvVarsCredentialsProvider())
300299
}
301300

302-
function makeEndpointsProvider(): EndpointsProvider {
301+
function makeEndpointsProvider() {
303302
const localManifestFetcher = new FileResourceFetcher(globals.manifestPaths.endpoints)
304303
const remoteManifestFetcher = new HttpResourceFetcher(endpointsFileUrl, { showUrl: true })
305304

306-
const provider = new EndpointsProvider(localManifestFetcher, remoteManifestFetcher)
307-
308-
return provider
305+
return {
306+
local: () => getEndpointsFromFetcher(localManifestFetcher),
307+
remote: () => getEndpointsFromFetcher(remoteManifestFetcher),
308+
}
309309
}
310310

311311
function recordToolkitInitialization(activationStartedOn: number, logger?: Logger) {

src/shared/regions/endpointsProvider.ts

Lines changed: 0 additions & 43 deletions
This file was deleted.

src/shared/regions/regionProvider.ts

Lines changed: 62 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ const localize = nls.loadMessageBundle()
1010

1111
import * as vscode from 'vscode'
1212
import { getLogger } from '../logger'
13-
import { Endpoints, Region } from './endpoints'
14-
import { EndpointsProvider } from './endpointsProvider'
13+
import { Endpoints, loadEndpoints, Region } from './endpoints'
1514
import { AWSTreeNodeBase } from '../treeview/nodes/awsTreeNodeBase'
1615
import { regionSettingKey } from '../constants'
1716
import { AwsContext } from '../awsContext'
1817
import { getIdeProperties, isCloud9 } from '../extensionUtilities'
18+
import { ResourceFetcher } from '../resourcefetcher/resourcefetcher'
1919

2020
export const defaultRegion = 'us-east-1'
2121
export const defaultPartition = 'aws'
@@ -120,6 +120,7 @@ export class RegionProvider {
120120
}
121121

122122
private loadFromEndpoints(endpoints: Endpoints) {
123+
this.regionData.clear()
123124
endpoints.partitions.forEach(partition => {
124125
partition.regions.forEach(region =>
125126
this.regionData.set(region.id, {
@@ -140,40 +141,71 @@ export class RegionProvider {
140141
})
141142
})
142143
})
144+
this.onDidChangeEmitter.fire()
143145
}
144146

145-
public static fromEndpointsProvider(endpointsProvider: EndpointsProvider): RegionProvider {
147+
/**
148+
* @param endpointsProvider.local Retrieves endpoints manifest from local sources available to the toolkit. Expected
149+
* to resolve fast, and is both a placeholder until the remote resources are loaded, and
150+
* is a fallback in case the toolkit is unable to load a remote resource
151+
* @param endpointsProvider.remote Retrieves endpoints manifest from remote host
152+
*/
153+
public static fromEndpointsProvider(endpointsProvider: {
154+
local: () => Endpoints | Promise<Endpoints>
155+
remote: () => Endpoints | Promise<Endpoints>
156+
}): RegionProvider {
146157
const instance = new this()
147158

148-
endpointsProvider
149-
.load()
150-
.then(endpoints => {
151-
instance.regionData.clear()
152-
instance.loadFromEndpoints(endpoints)
153-
instance.onDidChangeEmitter.fire()
154-
})
155-
.catch(err => {
156-
getLogger().error('Failure while loading Endpoints Manifest: %s', err)
157-
158-
vscode.window.showErrorMessage(
159-
`${localize(
160-
'AWS.error.endpoint.load.failure',
161-
'The {0} Toolkit was unable to load endpoints data.',
162-
getIdeProperties().company
163-
)} ${
164-
isCloud9()
165-
? localize(
166-
'AWS.error.impactedFunctionalityReset.cloud9',
167-
'Toolkit functionality may be impacted until the Cloud9 browser tab is refreshed.'
168-
)
169-
: localize(
170-
'AWS.error.impactedFunctionalityReset.vscode',
171-
'Toolkit functionality may be impacted until VS Code is restarted.'
172-
)
173-
}`
159+
async function load() {
160+
getLogger().info('endpoints: retrieving AWS endpoints data')
161+
instance.loadFromEndpoints(await endpointsProvider.local())
162+
163+
try {
164+
instance.loadFromEndpoints(await endpointsProvider.remote())
165+
} catch (err) {
166+
getLogger().warn(
167+
`endpoints: failed to load from remote source, region data may appear outdated: %s`,
168+
err
174169
)
175-
})
170+
}
171+
}
172+
173+
load().catch(err => {
174+
getLogger().error('Failure while loading Endpoints Manifest: %s', err)
175+
176+
vscode.window.showErrorMessage(
177+
`${localize(
178+
'AWS.error.endpoint.load.failure',
179+
'The {0} Toolkit was unable to load endpoints data.',
180+
getIdeProperties().company
181+
)} ${
182+
isCloud9()
183+
? localize(
184+
'AWS.error.impactedFunctionalityReset.cloud9',
185+
'Toolkit functionality may be impacted until the Cloud9 browser tab is refreshed.'
186+
)
187+
: localize(
188+
'AWS.error.impactedFunctionalityReset.vscode',
189+
'Toolkit functionality may be impacted until VS Code is restarted.'
190+
)
191+
}`
192+
)
193+
})
176194

177195
return instance
178196
}
179197
}
198+
199+
export async function getEndpointsFromFetcher(fetcher: ResourceFetcher): Promise<Endpoints> {
200+
const endpointsJson = await fetcher.get()
201+
if (!endpointsJson) {
202+
throw new Error('Failed to get resource')
203+
}
204+
205+
const endpoints = loadEndpoints(endpointsJson)
206+
if (!endpoints) {
207+
throw new Error('Failed to load endpoints')
208+
}
209+
210+
return endpoints
211+
}

src/test/awsExplorer/awsExplorer.test.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import * as assert from 'assert'
77
import * as sinon from 'sinon'
88
import { AwsExplorer } from '../../awsexplorer/awsExplorer'
99
import { RegionNode } from '../../awsexplorer/regionNode'
10-
import { EndpointsProvider } from '../../shared/regions/endpointsProvider'
1110
import { RegionProvider } from '../../shared/regions/regionProvider'
1211
import { FakeExtensionContext, FakeMemento } from '../fakeExtensionContext'
1312
import {
@@ -16,7 +15,6 @@ import {
1615
DEFAULT_TEST_REGION_NAME,
1716
} from '../shared/regions/testUtil'
1817
import { makeFakeAwsContextWithPlaceholderIds } from '../utilities/fakeAwsContext'
19-
import { stub } from '../utilities/stubber'
2018

2119
describe('AwsExplorer', function () {
2220
let sandbox: sinon.SinonSandbox
@@ -52,15 +50,14 @@ describe('AwsExplorer', function () {
5250

5351
it('refreshes when the Region Provider is updated', async function () {
5452
const fakeContext = await FakeExtensionContext.create()
55-
const endpointsProvider = stub(EndpointsProvider)
56-
endpointsProvider.load.resolves({ partitions: [] })
57-
58-
const regionProvider = RegionProvider.fromEndpointsProvider(endpointsProvider)
53+
const endpoints = Promise.resolve({ partitions: [] })
54+
const regionProvider = RegionProvider.fromEndpointsProvider({
55+
local: () => endpoints,
56+
remote: () => endpoints,
57+
})
5958
const awsExplorer = new AwsExplorer(fakeContext, regionProvider)
60-
6159
const refreshStub = sandbox.stub(awsExplorer, 'refresh')
62-
await endpointsProvider.load()
63-
60+
await endpoints
6461
assert.ok(refreshStub.calledOnce, 'expected AWS Explorer to refresh itself')
6562
})
6663
})

src/test/shared/regions/endpointsProvider.test.ts

Lines changed: 0 additions & 53 deletions
This file was deleted.

src/test/shared/regions/regionProvider.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,24 @@ const endpoints = {
5353
}
5454

5555
describe('RegionProvider', async function () {
56+
describe('fromEndpointsProvider', function () {
57+
it('pulls from the local source first and remote source later', async function () {
58+
const localEndpoints = Promise.resolve({ partitions: endpoints.partitions.slice(0, 1) })
59+
const remoteEndpoints = Promise.resolve(endpoints)
60+
const regionProvider = RegionProvider.fromEndpointsProvider({
61+
local: () => localEndpoints,
62+
remote: () => remoteEndpoints,
63+
})
64+
65+
await localEndpoints
66+
assert.ok(regionProvider.isServiceInRegion(serviceId, regionCode), 'Expected service to be in region')
67+
assert.strictEqual(regionProvider.getPartitionId('awscnregion1'), undefined)
68+
await remoteEndpoints
69+
assert.ok(regionProvider.isServiceInRegion(serviceId, regionCode), 'Expected service to be in region')
70+
assert.strictEqual(regionProvider.getPartitionId('awscnregion1'), 'aws-cn')
71+
})
72+
})
73+
5674
describe('isServiceInRegion', async function () {
5775
it('indicates when a service is in a region', async function () {
5876
const regionProvider = new RegionProvider(endpoints)

0 commit comments

Comments
 (0)