[hunterckx]

Re: #4561

Needs Findable update including the changes from DataBiosphere/findable-ui#1123 - Done

anvil-cmg, hca-dcp, and lungmap are given a Ky config that prevents retrying on responses that lack a value in the Retry-After header.

Notes:

• For the sake of running it in Jest, Ky is added to the transpiled packages in the Next config
• The default redirect handling from fetch is used, meaning that Retry-After will not be referenced for status 307

[NoopDog]

Thanks Hunter, this matches what Azul asked for in #4561: only retry Azul requests whose response carries a Retry-After header. Errors without it, including 400, 500, 502 and 504, now fail straight away, and requests with it wait the header's delay and retry up to 3 times. Putting it behind kyOptions, so only the Azul sites change, is nice and tidy, and the new tests fail against the old behaviour, so they're doing their job.

Approving the policy. A few things before this ships:

1. Bump findable-ui once #1123 is released. CI is red until then, because 57.0.0 doesn't have kyOptions.
2. Check Azul prod before the prod release. As of Azul's Oct 6 comment, Access-Control-Expose-Headers is only live in the lower deployments. If prod doesn't expose Retry-After when this ships, the browser can't see the header, and prod stops retrying 429s and 503s entirely.
3. One question for Azul: do API Gateway's own 502 and 504 responses include CORS headers? Those errors come from the Gateway, not Azul's code (504 timeouts, 502 "payload too large"). If they arrive without CORS headers, the browser hides them and fetch reports a plain network error, which Ky still retries (1s/3s/9s). That would retry exactly the 502s and 504s Hannes said not to. If they do have CORS headers, this PR is right as it is. If not, the cleaner fix is on Azul's side.
4. 307 with Retry-After. The browser follows a 307 immediately and doesn't wait for the header's delay. Azul's #7452 was going to test this against a local Data Browser, so it's worth confirming that immediate follow-up is fine with them.
5. Small: the PR body says "Re: #4561". If this finishes the issue, "Closes #4561" will close it on merge. Optionally, the two checks in constants.ts can be one isHTTPError(error) && !error.response.headers.get("Retry-After") ? false : undefined, and the empty-string Retry-After test covers a value Azul won't send.

Dismissing my approval until the findable-ui bump lands, since this repo doesn't require green checks to merge. The feedback above still stands, and I'll re-review after the bump.

[hunterckx]

@NoopDog Updates re: your list:

1. Findable is updated to latest! (57.2.0)
2. From what I can tell, the Access-Control-Expose-Headers change does appear to be on prod for both HCA and AnVIL
3. I've added tests for 502 and 504, the results of which seem to indicate that they will not be retried without Retry-After (fetch does not automatically throw for any response code)
4. I have not currently made any fix for this; this one is tricky because redirect behavior is built into JavaScript's fetch API, so adding a delay would require fully custom redirect handling
5. All left as-is

Also: I've given the Ky settings an explicit list of status codes that are candidates for retrying, so that the same list can be provided as the list of status codes for which Retry-After should be referenced to determine the delay

[Copilot]

🟢 Approval recommended

The reviewed changes have no identified blocking issues; only an optional test-label spelling correction remains.

1 open issue

• Fix typo in test suite name · New

What changed in this PR

This PR adds shared, header-driven retry settings for Azul API requests used by AnVIL, HCA DCP, and LungMAP.

Changes:

• Prevent retries for responses without a nonempty Retry-After header while preserving default network-error retries.
• Upgrade Findable UI to support the shared Ky configuration.
• Configure Jest to use real Ky and add retry-behavior tests.

File	Description
site-config/​lungmap/​dev/​config.ts	Applies shared Azul retry options.
site-config/​hca-dcp/​ma-dev/​config.ts	Applies shared Azul retry options.
site-config/​common/​constants.ts	Defines the header-driven retry policy.
site-config/​anvil-cmg/​dev/​config.ts	Applies shared Azul retry options.
package.json	Updates Findable UI and adds fetch mocking support.
package-lock.json	Locks updated dependencies.
next.config.mjs	Enables Ky transpilation for Jest compatibility.
jest.config.js	Removes the forced Ky mock mapping.
__tests__/​site-config/​azul-ky-options.test.ts	Tests response-header and network-error retry behavior.

_{Review effort: Balanced}

[NoopDog]

Thanks Hunter, nice work! A few small things:

1. The config can shrink to just shouldRetry (inline).
2. Optional: jest-fetch-mock is only used by this test. Node's built-in Response with jest.spyOn(globalThis, "fetch") would do the same without a new dependency.
3. Could you add a short "How to verify" to the description? For example: on dev, a 503 with Retry-After retries once after that delay, and a 500 isn't retried.

[NoopDog]

This could be just shouldRetry. statusCodes is the same as Ky's default list, and Azul's OpenAPI spec only documents Retry-After on 429 and 503, which are already in Ky's default afterStatusCodes. So KY_RETRY_STATUSES and both overrides can go.

Suggested change

	const KY_RETRY_STATUSES = [408, 413, 429, 500, 502, 503, 504];
	export const AZUL_KY_OPTIONS = {
	retry: {
	// Any status code that can be retried should have its retry delay based on Retry-After
	afterStatusCodes: KY_RETRY_STATUSES,
	shouldRetry({ error }): boolean | undefined {
	// If the Retry-After header exists and has content, use default behavior
	if (
	error instanceof HTTPError &&
	error.response.headers.get("Retry-After")
	) {
	return undefined;
	}
	// Otherwise, never retry requests that received responses, and use default behavior for those that didn't (e.g. due to network errors)
	return "response" in error ? false : undefined;
	},
	statusCodes: KY_RETRY_STATUSES,
	},
	} satisfies KyOptions;
	export const AZUL_KY_OPTIONS = {
	retry: {
	// Azul sends Retry-After on the responses it wants retried (429 and 503), which Ky's default afterStatusCodes already cover.
	// Never retry any other response, and keep the default for network errors.
	shouldRetry({ error }): boolean | undefined {
	return error instanceof HTTPError &&
	!error.response.headers.get("Retry-After")
	? false
	: undefined;
	},
	},
	} satisfies KyOptions;

[hunterckx]

I wanted to guarantee that Retry-After will be used even if something changes on the Azul end

[NoopDog]

With the slimmer config, the "absent" and "empty string" cases still make sense for every status, but "retries based on Retry-After" only needs 429 and 503, since Azul doesn't send it on 500, 502 or 504.