Skip to content

fix: findIndex missing return, isURL null guard, sanitizeString null guard #42037

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.

Sign up for GitHub

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

Closed
PedroHenrique0713 wants to merge 4 commits into from
Closed

fix: findIndex missing return, isURL null guard, sanitizeString null guard #42037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aa1f7ce
fix: utility bug fixes in entity naming, visibility check, and widget…
PedroHenrique0713 Jul 23, 2026
bafbb3f
fix: retryPromise silent failure and areArraysEqual false positives
PedroHenrique0713 Jul 23, 2026
2aaed07
fix: pushToArray mutation and getPathAndValueDiff logic errors
PedroHenrique0713 Jul 23, 2026
9eb177c
fix: findIndex return, isURL null guard, sanitizeString null guard
PedroHenrique0713 Jul 23, 2026
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
13 changes: 7 additions & 6 deletions app/client/src/utils/AppsmithUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ export const getNextEntityName = (
existingNames: string[],
startWithoutIndex?: boolean,
) => {
const regex = new RegExp(`^${prefix}(\\d+)$`);
const escapedPrefix = prefix.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const regex = new RegExp(`^${escapedPrefix}(\\d+)$`);

const usedIndices: number[] = existingNames.map((name) => {
if (name && regex.test(name)) {
Expand Down Expand Up @@ -96,7 +97,8 @@ export const getNextEntityName = (

export const getDuplicateName = (prefix: string, existingNames: string[]) => {
const trimmedPrefix = prefix.replace(/ /g, "");
const regex = new RegExp(`^${trimmedPrefix}(\\d+)$`);
const escapedPrefix = trimmedPrefix.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const regex = new RegExp(`^${escapedPrefix}(\\d+)$`);
const usedIndices: number[] = existingNames.map((name) => {
if (name && regex.test(name)) {
const matches = name.match(regex);
Expand Down Expand Up @@ -316,11 +318,12 @@ export const retryPromise = async (
if (shouldRetry(e)) {
setTimeout(async () => {
if (retriesLeft === 1) {
return Promise.reject({
reject({
code: ERROR_CODES.SERVER_ERROR,
message: createMessage(ERROR_500),
show: false,
});
return;
Comment on lines 318 to +326

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Ensure every retry path settles the promise.

When shouldRetry(e) returns false, the outer promise remains pending. Also, retriesLeft === 1 misses zero or negative values, which can schedule retries indefinitely. Reject non-retryable errors immediately, use retriesLeft <= 1, and pass shouldRetry through the recursive call at Line 330.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/client/src/utils/AppsmithUtils.tsx` around lines 318 - 326, Update the
retry logic in AppsmithUtils to immediately reject when shouldRetry(e) is false,
use retriesLeft <= 1 to terminate exhausted retries, and pass shouldRetry into
the recursive call so every retry path settles the promise.

}

// Passing on "reject" is the important part
Expand Down Expand Up @@ -436,9 +439,7 @@ export function areArraysEqual(arr1: string[], arr2: string[]) {
if (arr1.length !== arr2.length) return false;

// Because the array is frozen in strict mode, you'll need to copy the array before sorting it
if ([...arr1].sort().join(",") === [...arr2].sort().join(",")) return true;

return false;
return [...arr1].sort().every((val, i) => val === [...arr2].sort()[i]);
}

export enum DataType {
Expand Down
4 changes: 1 addition & 3 deletions app/client/src/utils/JSPaneUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ export const getDifferenceInJSCollection = (
);

if (updateExisting) {
const indexOfArchived = toBearchivedActions.findIndex((js) => {
js.id === updateExisting.id;
});
const indexOfArchived = toBearchivedActions.findIndex((js) => js.id === updateExisting.id);

//will be part of new nameChangedActions for now
toBeUpdatedActions.push({
Expand Down
1 change: 1 addition & 0 deletions app/client/src/utils/TypeHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const getType = (value: unknown) => {
};

export function isURL(str: string) {
if (typeof str !== "string") return false;
const pattern = new RegExp(
"^((blob:)?https?:\\/\\/)?" + //protocol
"((([a-z\\d]([a-z\\d-]*[a-z\\d])*)\\.)+[a-z]{2,}|" + // domain name
Expand Down
1 change: 1 addition & 0 deletions app/client/src/utils/URLUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ export function matchesURLPattern(url: string) {
}

export const sanitizeString = (str: string): string => {
if (!str) return "";
return str.toLowerCase().replace(/[^a-z0-9]/g, "_");
};
8 changes: 4 additions & 4 deletions app/client/src/utils/getPathAndValueFromActionDiffObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ export function getPathAndValueFromActionDiffObject(actionObjectDiff: any) {
(acc: string, item: number | string) => {
try {
if (typeof item === "string" && acc) {
acc += `${path}.${item}`;
acc += `.${item}`;
} else if (typeof item === "string" && !acc) {
acc += `${item}`;
} else acc += `${path}[${item}]`;
} else acc += `[${item}]`;

return acc;
} catch (error) {
Expand All @@ -60,9 +60,9 @@ export function getPathAndValueFromActionDiffObject(actionObjectDiff: any) {
);
// get value from diff object
value = actionObjectDiff[i]?.rhs;
return { path, value };
}

return { path, value };
}
return { path, value };
}
}
11 changes: 7 additions & 4 deletions app/client/src/utils/helpers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ function isElementVisibleInContainer(

// Calculate the percentage of the element that is visible
const elementArea = element.clientWidth * element.clientHeight;
if (elementArea === 0) return false;
const visiblePercentage = (visibleArea / elementArea) * 100;

// Return whether the visible percentage is greater than or equal to the desired percentage
Expand All @@ -327,6 +328,7 @@ function getWidgetElementToScroll(
canvasWidgets: CanvasWidgetsReduxState,
): HTMLElement | null {
const widget = canvasWidgets[widgetId];
if (!widget) return null;
const parentId = widget.parentId;

// If the widget doesn't have a parent, scroll to the widget itself
Expand Down Expand Up @@ -1315,10 +1317,11 @@ export function pushToArray(
arr1?: unknown[],
makeUnique = false,
) {
if (Array.isArray(arr1)) arr1.push(item);
else return [item];

if (makeUnique) return uniq(arr1);
if (Array.isArray(arr1)) {
const newArr = [...arr1, item];
if (makeUnique) return uniq(newArr);
return newArr;
} else return [item];

return arr1;
}
Expand Down