Skip to content

Commit 95c4716

Browse files
fix(config): fail builds if failed to fetch extensions (#6008)
* fix(config): fail builds if failed to fetch extensions * fix(config): return never if failed to fetch extensions * chore(config): use feature flag to error builds if failed to fetch extensions from jigsaw * chore(config): add debug logs * chore(config): clean up logs
1 parent 0a66a55 commit 95c4716

File tree

4 files changed

+130
-5
lines changed

4 files changed

+130
-5
lines changed

packages/config/src/api/site_info.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ export const getSiteInfo = async function ({
3636
offline = false,
3737
testOpts = {},
3838
siteFeatureFlagPrefix,
39+
featureFlags = {},
3940
}: GetSiteInfoOpts) {
4041
const { env: testEnv = false } = testOpts
42+
const errorOnExtensionFetchFail = featureFlags.error_builds_on_extension_fetch_fail
4143

4244
if (api === undefined || mode === 'buildbot' || testEnv) {
4345
const siteInfo: { id?: string; account_id?: string } = {}
@@ -46,7 +48,9 @@ export const getSiteInfo = async function ({
4648
if (accountId !== undefined) siteInfo.account_id = accountId
4749

4850
const integrations =
49-
mode === 'buildbot' && !offline ? await getIntegrations({ siteId, testOpts, offline, accountId }) : []
51+
mode === 'buildbot' && !offline
52+
? await getIntegrations({ siteId, testOpts, offline, accountId, errorOnExtensionFetchFail })
53+
: []
5054

5155
return { siteInfo, accounts: [], addons: [], integrations }
5256
}
@@ -55,7 +59,7 @@ export const getSiteInfo = async function ({
5559
getSite(api, siteId, siteFeatureFlagPrefix),
5660
getAccounts(api),
5761
getAddons(api, siteId),
58-
getIntegrations({ siteId, testOpts, offline, accountId }),
62+
getIntegrations({ siteId, testOpts, offline, accountId, errorOnExtensionFetchFail }),
5963
]
6064

6165
const [siteInfo, accounts, addons, integrations] = await Promise.all(promises)
@@ -109,13 +113,15 @@ type GetIntegrationsOpts = {
109113
accountId?: string
110114
testOpts: TestOptions
111115
offline: boolean
116+
errorOnExtensionFetchFail?: boolean
112117
}
113118

114119
const getIntegrations = async function ({
115120
siteId,
116121
accountId,
117122
testOpts,
118123
offline,
124+
errorOnExtensionFetchFail,
119125
}: GetIntegrationsOpts): Promise<IntegrationResponse[]> {
120126
if (!siteId || offline) {
121127
return []
@@ -130,14 +136,31 @@ const getIntegrations = async function ({
130136
? `${baseUrl}team/${accountId}/integrations/installations/meta/${siteId}`
131137
: `${baseUrl}site/${siteId}/integrations/safe`
132138

139+
if (errorOnExtensionFetchFail) {
140+
try {
141+
const response = await fetch(url)
142+
if (!response.ok) {
143+
throw new Error(`Unexpected status code ${response.status} from fetching extensions`)
144+
}
145+
const bodyText = await response.text()
146+
if (bodyText === '') {
147+
return []
148+
}
149+
150+
const integrations = await JSON.parse(bodyText)
151+
return Array.isArray(integrations) ? integrations : []
152+
} catch (error) {
153+
return throwUserError(
154+
`Failed retrieving extensions for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`,
155+
)
156+
}
157+
}
158+
133159
try {
134160
const response = await fetch(url)
135-
136161
const integrations = await response.json()
137162
return Array.isArray(integrations) ? integrations : []
138163
} catch (error) {
139-
// TODO: We should consider blocking the build as integrations are a critical part of the build process
140-
// https://linear.app/netlify/issue/CT-1214/implement-strategy-in-builds-to-deal-with-integrations-that-we-fail-to
141164
return []
142165
}
143166
}

packages/config/tests/api/snapshots/tests.js.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,6 +2250,56 @@ Generated by [AVA](https://avajs.dev).
22502250
"token": "test"␊
22512251
}`
22522252

2253+
## Integrations are not returned if failed to fetch integrations and if flag is true
2254+
2255+
> Snapshot 1
2256+
2257+
'Failed retrieving extensions for site test: Unexpected status code 500 from fetching extensions. Double-check your login status with \'netlify status\' or contact support with details of your error.'
2258+
2259+
## Empty array of integrations are returned if failed to fetch integrations and if flag is false
2260+
2261+
> Snapshot 1
2262+
2263+
`{␊
2264+
"accounts": [],␊
2265+
"addons": [],␊
2266+
"branch": "branch",␊
2267+
"buildDir": "packages/config/tests/api/fixtures/base",␊
2268+
"config": {␊
2269+
"build": {␊
2270+
"environment": {},␊
2271+
"processing": {␊
2272+
"css": {},␊
2273+
"html": {},␊
2274+
"images": {},␊
2275+
"js": {}␊
2276+
},␊
2277+
"publish": "packages/config/tests/api/fixtures/base",␊
2278+
"publishOrigin": "default",␊
2279+
"services": {}␊
2280+
},␊
2281+
"functions": {␊
2282+
"*": {}␊
2283+
},␊
2284+
"headers": [],␊
2285+
"plugins": [],␊
2286+
"redirects": []␊
2287+
},␊
2288+
"configPath": "packages/config/tests/api/fixtures/base/netlify.toml",␊
2289+
"context": "production",␊
2290+
"env": {},␊
2291+
"hasApi": true,␊
2292+
"headersPath": "packages/config/tests/api/fixtures/base/_headers",␊
2293+
"integrations": [],␊
2294+
"redirectsPath": "packages/config/tests/api/fixtures/base/_redirects",␊
2295+
"repositoryRoot": "packages/config/tests/api/fixtures/base",␊
2296+
"siteInfo": {␊
2297+
"account_id": "account1",␊
2298+
"id": "test"␊
2299+
},␊
2300+
"token": "test"␊
2301+
}`
2302+
22532303
## baseRelDir is true if build.base is overridden
22542304

22552305
> Snapshot 1
@@ -2413,3 +2463,9 @@ Generated by [AVA](https://avajs.dev).
24132463
},␊
24142464
"token": "test"␊
24152465
}`
2466+
2467+
## Integrations are not returned if failed to fetch integrations
2468+
2469+
> Snapshot 1
2470+
2471+
'Failed retrieving extensions for site test: Unexpected status code 500 from fetching extensions. Double-check your login status with \'netlify status\' or contact support with details of your error.'
123 Bytes
Binary file not shown.

packages/config/tests/api/tests.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ const TEAM_INSTALLATIONS_META_RESPONSE = {
3737
],
3838
}
3939

40+
const TEAM_INSTALLATIONS_META_RESPONSE_INTERNAL_SERVER_ERROR = {
41+
path: '/team/account1/integrations/installations/meta/test',
42+
response: { error: 'Internal Server Error' },
43+
status: 500,
44+
}
45+
4046
const SITE_INTEGRATIONS_EMPTY_RESPONSE = {
4147
path: '/site/test/integrations/safe',
4248
response: [],
@@ -413,6 +419,46 @@ test('Integrations are returned if accountId is present and mode is dev', async
413419
t.assert(config.integrations[0].has_build === true)
414420
})
415421

422+
test('Integrations are not returned if failed to fetch integrations and if flag is true', async (t) => {
423+
const { output } = await new Fixture('./fixtures/base')
424+
.withFlags({
425+
siteId: 'test',
426+
mode: 'buildbot',
427+
accountId: 'account1',
428+
token: 'test',
429+
featureFlags: {
430+
error_builds_on_extension_fetch_fail: true,
431+
},
432+
})
433+
.runConfigServer([
434+
SITE_INFO_DATA,
435+
TEAM_INSTALLATIONS_META_RESPONSE_INTERNAL_SERVER_ERROR,
436+
FETCH_INTEGRATIONS_EMPTY_RESPONSE,
437+
])
438+
439+
t.snapshot(normalizeOutput(output))
440+
})
441+
442+
test('Empty array of integrations are returned if failed to fetch integrations and if flag is false', async (t) => {
443+
const { output } = await new Fixture('./fixtures/base')
444+
.withFlags({
445+
siteId: 'test',
446+
mode: 'buildbot',
447+
accountId: 'account1',
448+
token: 'test',
449+
featureFlags: {
450+
error_builds_on_extension_fetch_fail: false,
451+
},
452+
})
453+
.runConfigServer([
454+
SITE_INFO_DATA,
455+
TEAM_INSTALLATIONS_META_RESPONSE_INTERNAL_SERVER_ERROR,
456+
FETCH_INTEGRATIONS_EMPTY_RESPONSE,
457+
])
458+
459+
t.snapshot(normalizeOutput(output))
460+
})
461+
416462
test('baseRelDir is true if build.base is overridden', async (t) => {
417463
const fixturesDir = normalize(`${fileURLToPath(test.meta.file)}/../fixtures`)
418464

0 commit comments

Comments
 (0)