Ag-Grid timeout error

[ducnva]

Description

Hi everyone,
I am encountering a timeout error when using the getDisplayedRowAtIndex API. Despite increasing the timeout to 600 seconds, I continue to encounter this error. What should I do? Can someone help me identify the cause of this error?"
rowNode = await grid.run_grid_method('getDisplayedRowAtIndex', 0, timeout= 600)
The message error:

JavaScript did not respond within 1.0 s
Traceback (most recent call last):
  File "/test/test/vendors/nicegui/nicegui/events.py", line 406, in wait_for_result
    await result
  File "/test/test/test.py", line 63, in detect_row_change
    rowNode = await grid.run_grid_method('getDisplayedRowAtIndex', rowIndex, timeout= 600)
  File "/test/test/vendors/nicegui/nicegui/client.py", line 211, in send_and_wait
    raise TimeoutError(f'JavaScript did not respond within {timeout:.1f} s')
TimeoutError: JavaScript did not respond within 600.0 s

[dr-yd]

Same issue here, for pretty much all grid API methods. (Not ALL of them, though - getDisplayedRowCount works in the same place.) Example code:

from nicegui import ui

grid = ui.aggrid({
    'columnDefs': [{'field': 'name', 'checkboxSelection': True}, { 'field': 'age'}, ],
    'rowData': [ {  'name': 'Alice',  'age': 18  },   {  'name': 'Bob', 'age': 21  }, ],
    ':getRowId':
    '(params) => params.data.name',
})
async def focus_handler(event):
    print(await grid.run_grid_method('getDisplayedRowAtIndex', event.args['rowIndex']))
grid.on('cellFocused', lambda e: focus_handler(e))
ui.run()

[falkoschindler]

Thanks for reporting this issue, @ducnva and @dr-yd!
There is a JavaScript error "Maximum call stack size exceeded", which could be caused by cycles in the return data. But I'm not sure.

[dr-yd]

For comparison, you can also try getFocusedCell, also breaks - maybe even just iterating through all the grid API methods and running them would emerge some kind of pattern?

[falkoschindler]

I assume every method returning cells or rows will have the same problem.

[lapnd]

This is because aggrid returns object that has recursive data. When sending data from js to python, socket.io need encode this object and due to the recurstion, the exeption happen.
In the templates/index.html

      function runJavascript(code, request_id) {
        (new Promise((resolve) =>resolve(eval(code)))).catch((reason) => {
          if(reason instanceof SyntaxError)
            return eval(`(async() => {${code}})()`);
          else
            throw reason;
        }).then((result) => {
          if (request_id) {
            window.socket.emit("javascript_response", {request_id, client_id: window.client_id, result});
          }
        });
      }

If you check the result object, you will see that.
Should we need somehow detect recursive data structure and omit them?
I'm not sure what is the best way to fix it yet

[lapnd]

My hot fix is

      function runJavascript(code, request_id) {
        (new Promise((resolve) =>resolve(eval(code)))).catch((reason) => {
          if(reason instanceof SyntaxError)
            return eval(`(async() => {${code}})()`);
          else
            throw reason;
        }).then((result) => {
          if (request_id) {
            try {
                window.socket.emit("javascript_response", {request_id, client_id: window.client_id, result});
            }catch (error) {
                if (error instanceof RangeError) {
                    // Handle RangeError (Maximum call stack size exceeded)
                    ['parent', 'beans'].forEach(prop => result?.hasOwnProperty(prop) && delete result[prop]);
                    // Emit the response without the problematic properties
                    window.socket.emit("javascript_response", { request_id, client_id: window.client_id, result });
                } else {
                    throw error; // Rethrow other types of errors
                }
            }
          }
        });
      }

I hope community can suggest the better way to fix this issue

[WolfgangFahl]

I also get frequent timeouts with the new run_method calls.

[dr-yd]

Can confirm that the hotfix works for my usage. In Firefox, InternalError has to be caught instead.

[dr-yd]

Spoke too soon, unfortunately... although I'm not sure whether the issue is caused by the quickfix or because my concept is wrong. My practical application is this callback:

class Grid:
    _pos: tuple[str, str]

        [... __init__]
            _ = self._ag_grid.on('cellFocused', lambda event: self.focused_callback(event))  # pylint: disable=unnecessary-lambda
        [...]


    async def focused_callback(self, event: GenericEventArguments):
        """
        Callback function for focused cell changes in the table.

        :param event: The event generated by the callback.
        """
        self.log.debug(f'Focused cell changed: {event.args}')
        row = await self._ag_grid.run_grid_method('getDisplayedRowAtIndex', event.args['rowIndex'])
        self._pos = (row['id'], event.args['colId'])
        self.log.debug(f'Focused cell is now {self._pos}')

This works for a while, but as soon as the edge of the displayed area is reached by scrolling with the keyboard, JS starts throwing errors again:

ag-grid-community.min.js:8 Uncaught TypeError: Cannot read properties of undefined (reading 'level')
    at jd.checkStickyRows (ag-grid-community.min.js:8:685220)
    at Ud.redraw (ag-grid-community.min.js:8:699545)
    at Ud.onBodyScroll (ag-grid-community.min.js:8:699245)
    at ag-grid-community.min.js:8:36985
    at Set.forEach (<anonymous>)
    at i (ag-grid-community.min.js:8:36942)
    at De.dispatchToListeners (ag-grid-community.min.js:8:37039)
    at De.dispatchEvent (ag-grid-community.min.js:8:36680)
    at Sl.fireScrollEvent (ag-grid-community.min.js:8:433476)
    at Sl.redrawRowsAfterScroll (ag-grid-community.min.js:8:434229)

And

ag-grid-community.min.js:8 Uncaught TypeError: Cannot read properties of undefined (reading 'gridOptionsService')
    at Ya.getValueFromValueService (ag-grid-community.min.js:8:309886)
    at _l.updateAndFormatValue (ag-grid-community.min.js:8:488444)
    at new _l (ag-grid-community.min.js:8:477190)
    at ag-grid-community.min.js:8:501954
    at Array.forEach (<anonymous>)
    at Wl.createCellCtrls (ag-grid-community.min.js:8:501895)
    at Wl.createAllCellCtrls (ag-grid-community.min.js:8:502969)
    at Wl.updateColumnListsImpl (ag-grid-community.min.js:8:502225)
    at ag-grid-community.min.js:8:501696
    at Vd.executeFrame (ag-grid-community.min.js:8:680509)

Individual cells are missing and unselectable, whole rows are missing, scrolling gets messed up etc. I don't know anything about the JS parts, unfortunately, so I can't say if this is possibly related to the proposed quickfix.

I also tried getFocusedCell instead of the callback because the callback idea was just a workaround. This didn't work at all - still threw the same error despite the attempt at catching it, presumably because there are additional keys in it that are recursive. I then asked ChatGPT and it gave me this to recursively remove recursion from the object:

function removeRecursiveReferences(obj, parent = null, key = null, visited = new WeakMap()) {
    if (obj && typeof obj === 'object') { // Check if it's an object or array
        if (visited.has(obj)) {
            // Recursive reference found, handle accordingly
            if (parent && key) {
                delete parent[key]; // Remove the recursive reference
            }
            return;
        }
        visited.set(obj, true); // Mark this object as visited

        Object.keys(obj).forEach(k => {
            removeRecursiveReferences(obj[k], obj, k, visited);
        });
    }
}

This operates on the return structure in-place. It worked correctly for the first cell, but afterwards, the entire table was broken. I also got a gigantic amount of data back from getFocusedCell that looked like it might be the entire state of the grid? It probably manipulated that in-place, as would the quickfix from above, which is why they might both break things. And cloning the object doesn't look simple or performant for something this big.

[pikaro]

I don't think it's really possible to solve this cleanly, isn't it? You'll either have to hardcode special treatment for AG Grid in the JS or make a general way to remove recursion available in the runJavascript method. I've added the latter and it works for my purposes:

#2671

Not sure if this is really usable because it's very opaque to the user. At least it would have to be documented that beans and parent have to be excluded for AG Grid methods that return rows / cols / cells, plus potentially some logging from JS when recursion does occur in other places. Ideally, you'd probably want to have sensible defaults for the exclusions in the AG Grid methods but that has a large potential for breaking things.

But just logically speaking, I don't think there are many options when the grid gives us recursive data. Trying to clone the object minus the recursive parts automatically works but returns 1 MB of data for a small grid for me regardless of the method so that's not usable at all.

[falkoschindler]

Thanks a lot for the suggestion and PR, @pikaro!

Here are my thoughts about our options so far:

1. Introduce a new parameter like in your PR Add return_exclusions to runJavascript() #2671:

   grid.run_grid_method('getDisplayedRowAtIndex', return_exclusions=[...])

   For this to work consistently across various elements, we need to extend the parameter list of many methods. Search "def run_" in nicegui/elements to get a list of run_method, run_grid_method, run_map_method, ...

2. You mentioned auto-removing circles. I would love to do that automatically. As you also noticed, the remaining payload might still be huge. But this is a slightly different, AG Grid-specific problem. Other methods could work well with this approach.

3. A special treatment for ui.aggrid is also an option. We actually already do that for ui.leaflet (see leaflet.js) or ui.plotly (see plotly.vue).

4. I wonder if we could introduce a new syntax like

   grid.run_grid_method('(grid) => grid.getDisplayedRowAtIndex(0)')

   The old API would still work, but you could also pass a JavaScript function that is interpreted on the client. This way you could specify the return value much more freely:

   grid.run_grid_method('(grid) => grid.getDisplayedRowAtIndex(0).id')

   or

   grid.run_grid_method('''(grid) => {
       const node = grid.getDisplayedRowAtIndex(0);
       return { id: node.id, foo: node.foo };
   }''')

[dr-yd]

@pikaro is also me, I just signed into the wrong account.

===

In order to make the behavior consistent, this would have to be extended to all methods, yes - I just avoided doing that before I knew if you would consider this a possible implementation. I can amend the PR to patch the other methods as well.

Regarding automatic recursion removal - I simply used ChatGPT code for that as well, but it wasn't performant at all due to the number of nodes being visited. I didn't put more time into it because it still leaves you with the issue that you'd have to pass all responses through it or change the API to make this optional. And if the API needs to be changed, an exclusion list is preferable IMO - or you could of course combine both.

Regarding special treatment of AG Grid - I noticed that the exact keys that need to be excluded are somewhat variable and probably hard to pin down. For getDisplayedRowAtIndex, sometimes there are beans and sometimes parent, and for getFocusedCell I had to exclude

            return_exclusions=[
                'column.eventService',
                'column.gridOptionsService',
                'column.columnUtils',
                'column.columnHoverService',
            ],

... because some of the data in column is necessary. It would probably be hard to maintain appropriate lists for all methods - I don't think that from the AG Grid team perspective, adding new recursive elements would be seen as breaking the API.

So this probably does mean that your suggestion in 4 would be the cleanest solution.

[falkoschindler]

I just experimented with a different function to stringify event arguments in index.html (inspired from here):

function getCircularReplacer() {
  const ancestors = [];
  return function (key, value) {
    if (typeof value !== "object" || value === null) {
      return value;
    }
    while (ancestors.length > 0 && ancestors.at(-1) !== this) {
      ancestors.pop();
    }
    if (ancestors.includes(value)) {
      return "[Circular]";
    }
    ancestors.push(value);
    return value;
  };
}

function stringifyEventArgs(args, event_args) {
  const result = [];
  args.forEach((arg, i) => result.push(JSON.stringify(arg, getCircularReplacer())));
  return result;
}

Even though this should remove circular references (and you can check by printing result), there's still a "RangeError: Maximum call stack size exceeded" on the JavaScript command line. And what's even weirder: When I remove the rowIndex from the event arguments

args[0].rowIndex = undefined;

before generating the result, the RangeError is gone. But when setting the rowIndex to something simply like 0, the RangeError is back.

So I don't quite understand where this error is coming from. And even though we could improve how event arguments are filtered in general, we need to find and fix the issue with getDisplayedRowAtIndex and its event arguments.

---

Edit:

The RangeError occurs even if I replace the function with this minimal implementation:

function stringifyEventArgs(args, event_args) {
  return ['{"rowIndex":0}'];
}

[falkoschindler]

Oh, in my previous comment I totally confused event arguments with the result of run_method. For event arguments there is already some sanitization, but not for the latter.

[falkoschindler]

A good location to uncycle messages to the server would probably be here:

nicegui/nicegui/templates/index.html

Line 226 in 24e15c4

window.socket.emit("javascript_response", {request_id, client_id: window.client_id, result});

But the circular reference replacer from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value#circular_references doesn't seem to work.

This brings me back to option 4, which is an API extension for running JavaScript methods:

grid.run_grid_method('(grid) => grid.getDisplayedRowAtIndex(0)')