[Bug]: Browser console returns a 404 when switching between pages of an app

[ramsaptami]

Is there an existing issue for this?

• [X] I have searched the existing issues

Description

Seeing a 404 Page not Found on the browser console while switching between pages of an app. The save state of the app goes from the green checkmark icon to a yellow error state when this happens. This is causing app flow to break since URL query param is being used to run queries which are in turn failing.

Steps To Reproduce

1. Go to the app at this location
2. Observe the save state of the app and keep the browser console open
3. Now switch between the home page and Spotify and Youtube page
4. Switch between objects/queries/add a comment line to trigger save action if nothing is happening
5. Observe the browser console display page not found error and save state of the app in error.

https://www.loom.com/share/cf230cdea7604963a08f71c67f08edd0

Public Sample App

https://release.app.appsmith.com/app/earworm/home-6219207a0276eb01d22fec5e/edit

Version

Cloud

[trishaanand]

I triaged this with the help of @ramsaptami. Here are the findings :

The page save API URL is incorrect. The page id is correct but the layout id to which it is trying to save it is actually from the previous page (the page from where we switched to the current one). So, lets suppose we are on page 1. Page 1 has id p1 and layout id l1 Now we switch to page 2. Page 2 has id p2 and layout id l2. When triggering a page save call (which btw should not have been triggered at all, but story for another day), the client has constructed the URL for PUT on page 2 as : /layouts/l1/pages/p2

This instead should have been /layouts/l2/pages/p2

This is the reason for the server throwing a 404. Needs to be triaged now from the FE perspective around the incorrect URL construction.

[eco-monk]

Repro steps: (on prod and release)

1. Create multiple pages.
2. Drag and drop list widget into each page.
3. Switch between pages.

---

On page load - Page save is triggered whenever a list widget is present.

https://www.loom.com/share/47d6eb8a3ef24662bf1fb4ac2eca10bf

[rahulramesha]

RCA for this issue can be broken into 4 parts,

• While building urls, we get layoutId and pageId from 2 different sources, https://github.com/appsmithorg/appsmith/blob/c3bc8ae89bbaa7784f62ddb4c44d1074d6206374/app/client/src/sagas/selectors.tsx#L56 Here, layoutId is selected from state.ui.editor.currentLayoutId, while pageId is selected from state.entities.pageList.currentPageId.
• These two are updated at different times, state.entities.pageList.currentPageId is set when a differentPage is clicked from the entity explorer, while state.ui.editor.currentLayoutId is set after a backend call is made and the response is received shortly after.
• So, if a url is generated when in between the two updates, it would have nextPage's pageId and currentPage's layoutId. There usually would have been no conflict in the two ids, since there would have been no saveLayout call in between those two actions, But the savePageSaga is triggered inside a debounce, https://github.com/appsmithorg/appsmith/blob/c3bc8ae89bbaa7784f62ddb4c44d1074d6206374/app/client/src/sagas/PageSagas.tsx#L1072 Hence, even if a save action was triggered before clicking on the pageLink, savePageSaga is triggered only after 500ms, which could put in in between the updates of pageId and layoutId.

But the above mentioned logic has been working this way for a really long time, only related piece of code that was changed relatively recently was made in January where we trigger the save page call on page load from inside a List Widget.

List Widget on mount will trigger a save action here https://github.com/appsmithorg/appsmith/blob/c3bc8ae89bbaa7784f62ddb4c44d1074d6206374/app/client/src/widgets/ListWidget/widget/index.tsx#L87 that was added in the commit, https://github.com/appsmithorg/appsmith/commit/f7ce277e4cd6b6707cdb04be8515fa155a5647fc.

This would cause incorrect url to be called for saving the layout.

cc: @ohansFavour @somangshu @riodeuno

[somangshu]

@rahulramesha @ohansFavour do we need to think about this in the #7028?

cc @Tooluloope @ashit-rath

[rahulramesha]

@somangshu Even though it showed some gaps in the original Saving logic which we have to rethink, we definitely have to change the List Widget logic where we are updating the DSL on mount. We can use other temporary states rather than updating and saving the DSL every time List Widget is mounted.

[ashit-rath]

I think we can avoid it on mount as an existing list widget would have a list of private widgets and a new list widget drop we could use the blueprint operations to add the default widgets to the private widgets list.

@Tooluloope Let's add this to the list widget test plan.