Skip to content

Fix missing results from deduplicated calls to ListElementChildren

Erwan Rouchet requested to merge fix-300 into master

Closes #300 (closed)

This TypeError occured when a commit('elementsv2/addChildren', { element: '<uuid>', children: undefined }) occured, inside of the nextChildren action. After committing the children, this action would remove the results and insert all the other keys as the pagination, using delete data.results.

data is a variable that was retrieved from api.listElementChildren and held the response data. Duplicate calls to listElementChildren with the same arguments are deduplicated when a request is already in progress, to prevent making the same request multiple times, using the unique helper in js/api.js. However, this causes Promises to be passed around by reference, as well as response data; therefore, if nextChildren is called twice, their data will have the same reference. This causes delete data.results, in the first API call, to remove the children for all the other simultaneous calls, causing one or more calls to trigger addChildren with children: undefined.

I managed to reproduce the issue by adding a beautiful time.sleep(3) in the backend to slow the requests down:

  1. Open an element with a folder type
    • elementsv2/nextChildren is called in the background
    • The children tree is not displayed
  2. Before the children are loaded, open the tree manually
    • A watcher on the opened state is triggered on the tree
    • elementsv2/nextChildren is called again
  3. The children request is returned and both calls to nextChildren receive its data as data
  4. One of the calls triggers the addChildren mutation, then removes results from data
  5. The other call tries to trigger addChildren, but data.results is undefined, causing the TypeError.

Fixes:

  • Added a shallow clone of data to prevent the removal from affecting other calls
  • Added a check to addChildren to just make it ignore undefined
  • Added a check on the ElementTree to try to prevent calling nextChildren if it is already pending
  • Added a try..finally to ensure a user can retry loading the children after an error occurs
  • Removed a useless mapMutation that caused a small confusion while troubleshooting
Edited by Erwan Rouchet

Merge request reports

Loading