Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions workspaces/orchestrator/.changeset/widget-fetch-retry.md
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
24 changes: 24 additions & 0 deletions workspaces/orchestrator/docs/orchestratorFormWidgets.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ The widget supports following `ui:props`:
- fetch:method
- fetch:body
- fetch:retrigger
- fetch:retry:maxAttempts
- fetch:retry:delay
- fetch:retry:backoff
- fetch:retry:statusCodes
- fetch:error:ignoreUnready
- fetch:error:silent
- fetch:response:value
Expand Down Expand Up @@ -301,6 +305,10 @@ The widget supports following `ui:props`:
- fetch:method
- fetch:body
- fetch:retrigger
- fetch:retry:maxAttempts
- fetch:retry:delay
- fetch:retry:backoff
- fetch:retry:statusCodes
- fetch:error:ignoreUnready
- fetch:error:silent
- fetch:skipInitialValue
Expand Down Expand Up @@ -349,6 +357,10 @@ The widget supports following `ui:props`:
- fetch:method
- fetch:body
- fetch:retrigger
- fetch:retry:maxAttempts
- fetch:retry:delay
- fetch:retry:backoff
- fetch:retry:statusCodes
- fetch:error:ignoreUnready
- fetch:error:silent
- fetch:skipInitialValue
Expand Down Expand Up @@ -400,6 +412,10 @@ The widget supports following `ui:props`:
- fetch:method
- fetch:body
- fetch:retrigger
- fetch:retry:maxAttempts
- fetch:retry:delay
- fetch:retry:backoff
- fetch:retry:statusCodes
- fetch:error:ignoreUnready
- fetch:error:silent
- fetch:skipInitialValue
Expand Down Expand Up @@ -522,6 +538,10 @@ The widget supports the following `ui:props` (for detailed information on each,
- `fetch:body`: HTTP body for the fetch request
- `fetch:retrigger`: Array of field paths that trigger a refetch when their values change
- `fetch:clearOnRetrigger`: Clears the field value when retrigger dependencies change
- `fetch:retry:maxAttempts`: Enables retry logic for the fetch call
- `fetch:retry:delay`: Initial delay (ms) before the first retry
- `fetch:retry:backoff`: Backoff multiplier applied to the delay
- `fetch:retry:statusCodes`: Optional list of status codes to retry
Comment on lines +541 to +544
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Missing retry ui:props example 📎 Requirement gap ⚙ Maintainability

Documentation lists the new fetch:retry:* properties but does not include an example snippet
showing how to configure them via ui:props. This violates the documentation requirement and makes
correct usage harder to discover.
Agent Prompt
## Issue description
Documentation mentions the `fetch:retry:*` properties but does not include an example `ui:props` snippet showing how to configure them.

## Issue Context
Compliance requires docs to include an example configuration for the new retry props.

## Fix Focus Areas
- workspaces/orchestrator/docs/orchestratorFormWidgets.md[541-568]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


## Content of `ui:props`

Expand All @@ -542,6 +562,10 @@ Various selectors (like `fetch:response:*`) are processed by the [jsonata](https
| fetch:body | An object representing the body of an HTTP POST request. Not used with the GET method. Property value can be a string template or an array of strings. templates. | `{“foo”: “bar $${{identityApi.token}}”, "myArray": ["constant", "$${{current.solutionName}}"]}` |
| fetch:retrigger | An array of keys/key families as described in the Backstage API Exposed Parts. If the value referenced by any key from this list is changed, the fetch is triggered. | `["current.solutionName", "identityApi.profileName"]` |
| fetch:clearOnRetrigger | When set to `true`, clears the field value as soon as any `fetch:retrigger` dependency changes, before the fetch completes. Useful to avoid stale values while refetching. | `true`, `false` (default: `false`) |
| fetch:retry:maxAttempts | Enables retry logic for the widget fetch. The value is the maximum number of retries after the initial failure. If not set or `0`, retries are disabled. | `3` (retries) |
| fetch:retry:delay | Initial delay in milliseconds before the first retry. Combined with `fetch:retry:backoff` for exponential backoff. | `1000` (ms) |
| fetch:retry:backoff | Multiplier applied to the delay for each subsequent retry. Use `2` for exponential backoff, or `1` for fixed delay. | `2` |
| fetch:retry:statusCodes | Optional list of HTTP status codes that should be retried. If omitted, any non-OK response is eligible for retry. | `[408, 429, 500, 502, 503, 504]` |
| fetch:error:ignoreUnready | When set to `true`, suppresses fetch error display until all `fetch:retrigger` dependencies have non-empty values. This is useful when fetch depends on other fields that are not filled yet, preventing expected errors from being displayed during initial load. | `true`, `false` (default: `false`) |
| fetch:error:silent | When set to `true`, suppresses fetch error display when the fetch request returns a non-OK status (4xx/5xx). Use this when you want to handle error states via conditional UI instead of showing the widget error. | `true`, `false` (default: `false`) |
| fetch:skipInitialValue | When set to `true`, prevents applying the initial value from `fetch:response:value`, keeping the field empty until the user selects or types a value. | `true`, `false` (default: `false`) |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export type UiProps = {
'fetch:body'?: Record<string, JsonValue>;
'fetch:retrigger'?: string[];
'fetch:clearOnRetrigger'?: boolean;
'fetch:retry:maxAttempts'?: number;
'fetch:retry:delay'?: number;
'fetch:retry:backoff'?: number;
'fetch:retry:statusCodes'?: number[];
'fetch:error:ignoreUnready'?: boolean;
'fetch:error:silent'?: boolean;
'fetch:skipInitialValue'?: boolean;
Expand Down
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

3. Status codes type mismatch 🐞 Bug ✓ Correctness

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
### Issue description
`fetch:retry:statusCodes` can arrive as strings at runtime, but `fetchWithRetry` compares them to numeric `response.status`, causing mismatches.

### Issue Context
Even though `UiProps` types it as `number[]`, runtime config can bypass TypeScript.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/retry.ts[44-53]

### Suggested fix
- Normalize `statusCodes` by mapping to numbers and filtering invalid values:
  - `const codes = Array.isArray(options.statusCodes) ? options.statusCodes.map(Number).filter(Number.isFinite) : []`
  - build `new Set(codes)` only if `codes.length > 0`.
- Optionally `Math.trunc` each code and clamp to valid HTTP range (100-599) if desired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 20 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=redhat-developer_rhdh-plugins&issues=AZ0kQIn4JZM6GU0k-EcI&open=AZ0kQIn4JZM6GU0k-EcI&pullRequest=2601
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
Expand Up @@ -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.
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Retries ignore cancellation 🐞 Bug ⛯ Reliability

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
### Issue description
`fetchWithRetry` has no cancellation mechanism. `useFetch` uses `requestIdRef` to prevent stale state updates, but retries still execute after a new request starts or after unmount.

### Issue Context
This can create redundant network calls and pending timeouts when `fetch:retry:*` is enabled and inputs change quickly.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useFetch.ts[153-208]
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/retry.ts[71-117]

### Suggested fix
- Introduce an `AbortController` per request in `useFetch` and pass `signal` into `fetchApi.fetch` (extend `evaluatedRequestInit` with `{ signal }`).
- Extend `fetchWithRetry` to accept an `AbortSignal` (or a `shouldStop()` callback) and:
  - Immediately stop retries when aborted.
  - Avoid sleeping when aborted.
  - Avoid retrying `AbortError`.
- Ensure cleanup aborts the in-flight request when dependencies change/unmount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

if (!response.ok) {
throw new Error(
Expand Down Expand Up @@ -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,
],
Expand Down
Loading