-
Notifications
You must be signed in to change notification settings - Fork 95
feat(orchestrator-form-widgets): add widget fetch retries #2601
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@red-hat-developer-hub/backstage-plugin-orchestrator-form-widgets': patch | ||
| --- | ||
|
|
||
| add configurable retry options for widget fetch requests |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| /* | ||
| * Copyright Red Hat, Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| export type FetchRetryOptions = { | ||
| maxAttempts?: number; | ||
| delayMs?: number; | ||
| backoff?: number; | ||
| statusCodes?: number[]; | ||
| }; | ||
|
|
||
| const sleep = (ms: number) => | ||
| new Promise<void>(resolve => { | ||
| setTimeout(resolve, ms); | ||
| }); | ||
|
|
||
| const DEFAULT_RETRY_DELAY_MS = 1000; | ||
| const DEFAULT_RETRY_BACKOFF = 2; | ||
|
|
||
| const normalizeRetryOptions = (options?: FetchRetryOptions) => { | ||
| if (!options) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const maxAttempts = Number(options.maxAttempts); | ||
| if (!Number.isFinite(maxAttempts) || maxAttempts <= 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const delayMs = Number(options.delayMs ?? DEFAULT_RETRY_DELAY_MS); | ||
| const backoff = Number(options.backoff ?? DEFAULT_RETRY_BACKOFF); | ||
| const statusCodes = | ||
| Array.isArray(options.statusCodes) && options.statusCodes.length > 0 | ||
| ? new Set(options.statusCodes) | ||
| : undefined; | ||
|
Comment on lines
+44
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3. Status codes type mismatch fetchWithRetry stores statusCodes as-is in a Set, so string values like "500" will never match numeric Response.status and configured retries will be silently skipped. This breaks the documented behavior when retry options come from untyped JSON/YAML sources. Agent Prompt
|
||
|
|
||
| return { | ||
| maxAttempts: Math.floor(maxAttempts), | ||
| delayMs: Math.max(0, delayMs), | ||
| backoff: Math.max(1, backoff), | ||
| statusCodes, | ||
| }; | ||
| }; | ||
|
|
||
| const getErrorStatus = (error: unknown): number | undefined => { | ||
| if (!error || typeof error !== 'object') { | ||
| return undefined; | ||
| } | ||
|
|
||
| const maybeError = error as { | ||
| status?: unknown; | ||
| response?: { status?: unknown }; | ||
| }; | ||
|
|
||
| const status = maybeError.status ?? maybeError.response?.status; | ||
| return typeof status === 'number' ? status : undefined; | ||
| }; | ||
|
|
||
| export const fetchWithRetry = async ( | ||
| fetchFn: () => Promise<Response>, | ||
| options?: FetchRetryOptions, | ||
| ): Promise<Response> => { | ||
|
Check failure on line 74 in workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/retry.ts
|
||
| const retryOptions = normalizeRetryOptions(options); | ||
| if (!retryOptions) { | ||
| return fetchFn(); | ||
| } | ||
|
|
||
| for (let attempt = 0; attempt <= retryOptions.maxAttempts; attempt += 1) { | ||
| try { | ||
| const response = await fetchFn(); | ||
| if (response.ok) { | ||
| return response; | ||
| } | ||
|
|
||
| if ( | ||
| retryOptions.statusCodes && | ||
| !retryOptions.statusCodes.has(response.status) | ||
| ) { | ||
| return response; | ||
| } | ||
|
|
||
| if (attempt >= retryOptions.maxAttempts) { | ||
| return response; | ||
| } | ||
| } catch (error) { | ||
| const status = getErrorStatus(error); | ||
| if ( | ||
| status !== undefined && | ||
| retryOptions.statusCodes && | ||
| !retryOptions.statusCodes.has(status) | ||
| ) { | ||
| throw error; | ||
| } | ||
|
|
||
| if (attempt >= retryOptions.maxAttempts) { | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| const waitMs = | ||
| retryOptions.delayMs * Math.pow(retryOptions.backoff, attempt); | ||
| if (waitMs > 0) { | ||
| await sleep(waitMs); | ||
| } | ||
| } | ||
|
|
||
| throw new Error('Retry attempts exceeded without a terminal response.'); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ import { useRetriggerEvaluate } from './useRetriggerEvaluate'; | |
| import { useDebounce } from 'react-use'; | ||
| import { DEFAULT_DEBOUNCE_LIMIT } from '../widgets/constants'; | ||
| import isEqual from 'lodash/isEqual'; | ||
| import { fetchWithRetry } from './retry'; | ||
|
|
||
| /** | ||
| * Checks if all fetch:retrigger dependencies have non-empty values. | ||
|
|
@@ -57,6 +58,10 @@ export const useFetch = ( | |
| const fetchUrl = uiProps['fetch:url']; | ||
| const skipErrorWhenDepsEmpty = uiProps['fetch:error:ignoreUnready'] === true; | ||
| const clearOnRetrigger = uiProps['fetch:clearOnRetrigger'] === true; | ||
| const retryMaxAttempts = uiProps['fetch:retry:maxAttempts']; | ||
| const retryDelay = uiProps['fetch:retry:delay']; | ||
| const retryBackoff = uiProps['fetch:retry:backoff']; | ||
| const retryStatusCodes = uiProps['fetch:retry:statusCodes']; | ||
| const evaluatedRequestInit = useRequestInit({ | ||
| uiProps, | ||
| prefix: 'fetch', | ||
|
|
@@ -103,6 +108,12 @@ export const useFetch = ( | |
|
|
||
| const hasFetchInputs = | ||
| !!fetchUrl && !!evaluatedFetchUrl && !!evaluatedRequestInit && !!retrigger; | ||
| const retryOptions = { | ||
| maxAttempts: retryMaxAttempts, | ||
| delayMs: retryDelay, | ||
| backoff: retryBackoff, | ||
| statusCodes: retryStatusCodes, | ||
| }; | ||
|
|
||
| // Set loading immediately on dependency changes so UI shows a spinner during debounce. | ||
| useEffect(() => { | ||
|
|
@@ -158,9 +169,9 @@ export const useFetch = ( | |
|
|
||
| setLoading(true); | ||
|
|
||
| const response = await fetchApi.fetch( | ||
| evaluatedFetchUrl, | ||
| evaluatedRequestInit, | ||
| const response = await fetchWithRetry( | ||
| () => fetchApi.fetch(evaluatedFetchUrl, evaluatedRequestInit), | ||
| retryOptions, | ||
| ); | ||
|
Comment on lines
+172
to
175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Retries ignore cancellation useFetch() guards state updates with requestIdRef, but fetchWithRetry() continues sleeping and issuing retries even after dependencies change or the component unmounts, causing unnecessary network traffic and timers. This can also amplify load if the user rapidly changes retrigger inputs while retries are enabled. Agent Prompt
|
||
| if (!response.ok) { | ||
| throw new Error( | ||
|
|
@@ -204,6 +215,10 @@ export const useFetch = ( | |
| evaluatedRequestInit, | ||
| fetchApi, | ||
| fetchUrl, | ||
| retryMaxAttempts, | ||
| retryDelay, | ||
| retryBackoff, | ||
| retryStatusCodes, | ||
| // no need to expand the "retrigger" array here since its identity changes only if an item changes | ||
| retrigger, | ||
| ], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Missing retry ui:props example
📎 Requirement gap⚙ MaintainabilityAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools