Skip to content

fix: empty catch blocks, falsy JSON escape, and stale closure in workers #42039

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 6 commits into from
Closed

fix: empty catch blocks, falsy JSON escape, and stale closure in workers #42039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 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
4acd54f
fix(formSelectors): guard against missing action with empty string key
PedroHenrique0713 Jul 23, 2026
c0861e8
fix: empty catch blocks, falsy JSON escape, and stale closure in workers
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
6 changes: 4 additions & 2 deletions app/client/src/selectors/formSelectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ export const getFormConfigConditionalOutput = (
): ConditionalOutput => {
const baseActionId = getActionIdFromURL();
const action = getActionByBaseId(state, baseActionId as string);
const actionId = action?.id ?? "";
const actionId = action?.id;
if (!actionId) return {} as ConditionalOutput;
const conditionalOutput = extractConditionalOutput(
config,
state.evaluations.triggers[actionId],
Expand All @@ -73,7 +74,8 @@ export const getDynamicFetchedValues = (
): DynamicValues => {
const baseActionId = getActionIdFromURL();
const action = getActionByBaseId(state, baseActionId as string);
const actionId = action?.id ?? "";
const actionId = action?.id;
if (!actionId) return {} as DynamicValues;
Comment on lines +77 to +78

@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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Return a safe DynamicValues shape for missing actions.

Casting {} to DynamicValues hides missing fields. app/client/src/components/formControls/SegmentedControl.tsx assigns dynamicFetchedValues.data directly to options, so this path changes the initialized [] to undefined when actionId is unavailable. Return typed defaults or coalesce these fields in the consumer.

As per coding guidelines, ensure TypeScript types are correct; this cast masks the required DynamicValues fields.

🤖 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/selectors/formSelectors.ts` around lines 77 - 78, Update the
missing-action branch in the selector containing actionId to return a fully
typed DynamicValues default with safe initialized values, especially data as an
empty array, instead of casting an empty object. Preserve the existing consumer
contract in SegmentedControl so dynamicFetchedValues.data remains defined when
no action ID is available.

Source: Coding guidelines

const conditionalOutput = extractConditionalOutput(
config,
state.evaluations.triggers[actionId],
Expand Down
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;
}

// 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);

@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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Format the changed predicate with Prettier.

Line 103 is overlong and should be wrapped to satisfy the repository’s formatting check.

As per coding guidelines, files under app/client/{src,cypress}/**/*.{ts,tsx,js} must pass Prettier via yarn run prettier.

Suggested formatting 
-          const indexOfArchived = toBearchivedActions.findIndex((js) => js.id === updateExisting.id);
+          const indexOfArchived = toBearchivedActions.findIndex(
+            (js) => js.id === updateExisting.id,
+          );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const indexOfArchived = toBearchivedActions.findIndex((js) => js.id === updateExisting.id);
const indexOfArchived = toBearchivedActions.findIndex(
(js) => js.id === updateExisting.id,
);
🤖 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/JSPaneUtils.tsx` at line 103, Format the findIndex
predicate in the updateExisting flow using the repository’s Prettier
configuration so the declaration is wrapped to the expected line length; do not
change its behavior.

Source: Coding guidelines


//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
9 changes: 7 additions & 2 deletions app/client/src/workers/Evaluation/evaluationSubstitution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,13 @@ export const templateSubstituteDynamicValues = (
}

try {
if (typeof value === "string" && JSON.parse(value)) {
value = value.replace(/\\([\s\S])|(")/g, "\\$1$2");
if (typeof value === "string") {
try {
JSON.parse(value);
value = value.replace(/\\([\s\S])|(")/g, "\\$1$2");
} catch (e) {
// not valid JSON — no escaping needed
}
}
} catch (e) {
// do nothing
Expand Down
4 changes: 3 additions & 1 deletion app/client/src/workers/Evaluation/formEval.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,9 @@ function evaluateFormConfigElements(
const evaluatedVal = eval(expression);

config[path].output = evaluatedVal;
} catch (e) {}
} catch (e) {
// evaluation error — config keeps its default/current value
}
});
}

Expand Down
4 changes: 3 additions & 1 deletion app/client/src/workers/Evaluation/handlers/jsLibrary.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,9 @@ function generateUniqueAccessor(

name = urlPathParts.pop() as string;
name = name?.includes("+esm") ? (urlPathParts.pop() as string) : name;
} catch (e) {}
} catch (e) {
/* not a valid URL — use raw name */
}

// Replace all non-alphabetic characters with underscores and remove trailing underscores
const validVar = name.replace(/[^a-zA-Z]/g, "_").replace(/_+$/, "");
Expand Down