From cca8d91b5681d7bc4f5fd465957326ec8d33eb83 Mon Sep 17 00:00:00 2001 From: user77 Date: Tue, 14 Jul 2026 15:11:05 +0500 Subject: [PATCH] fix(web): avoid infinite componentDidUpdate loop in the event log (#8312) * fix(web): avoid infinite componentDidUpdate loop in the event log EventList.componentDidUpdate called this.onViewportUpdate() on every update, even when the inputs to calcVScroll (the event list and the row height) had not changed. onViewportUpdate reads the current scroll position, recomputes vScroll, and setStates when the window shape changes. Because the event log measures per-row heights, a setState it produced could feed itself: rendering a new window remeasured rows, which shifted the computed window, which setState again, so the cycle never converged and surfaced as "Maximum update depth exceeded" while scrolling the log. Gate the call on prevProps.events !== this.props.events || prevProps.rowHeight !== this.props.rowHeight, mirroring the FlowTable fix in #8233. The other call sites (componentDidMount, the resize listener, the viewport onScroll, and setHeight when a row is first measured) still drive updates as needed, so scrolling and autoscroll keep working. Adds a regression test that fails without the gate. * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Maximilian Hils --- CHANGELOG.md | 2 + .../components/EventLog/EventListSpec.tsx | 39 +++++++++++++++++++ web/src/js/components/EventLog/EventList.tsx | 19 ++++++++- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 683315bc2..baa6ca56b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ ## Unreleased: mitmproxy next +- mitmweb: Fix an infinite update cycle in the event log by only recomputing the virtual-scroll window in `componentDidUpdate` when the event list or `rowHeight` actually change. + ([#8312](https://github.com/mitmproxy/mitmproxy/pull/8312), @hexbinoct) - Remove the unused `msgpack` dependency. The msgpack contentview is implemented in Rust and shipped with `mitmproxy_rs` since mitmproxy 12. ([#8319](https://github.com/mitmproxy/mitmproxy/pull/8319), @lukehsiao) diff --git a/web/src/js/__tests__/components/EventLog/EventListSpec.tsx b/web/src/js/__tests__/components/EventLog/EventListSpec.tsx index b6362821d..892a6e062 100644 --- a/web/src/js/__tests__/components/EventLog/EventListSpec.tsx +++ b/web/src/js/__tests__/components/EventLog/EventListSpec.tsx @@ -17,4 +17,43 @@ describe("EventList Component", () => { expect(asFragment()).toMatchSnapshot(); unmount(); // no errors }); + + it("does not call onViewportUpdate when events and rowHeight are unchanged", () => { + // Regression guard for an infinite componentDidUpdate -> setState + // cycle (React "Maximum update depth exceeded" when scrolling the + // event log). Before the props-comparison gate in componentDidUpdate, + // onViewportUpdate ran on EVERY update, including the setState the + // previous call itself produced, so with variable row heights the + // recomputed vScroll could ping-pong between windows and never + // converge. The same fix was applied to FlowTable in #8233. + const spy = jest.spyOn(EventLogList.prototype, "onViewportUpdate"); + const { rerender } = render(); + spy.mockClear(); // ignore the componentDidMount call + + // Re-render with the same events reference and rowHeight. This + // triggers componentDidUpdate but changes none of onViewportUpdate's + // inputs, so it must not run. + rerender(); + expect(spy).not.toHaveBeenCalled(); + + spy.mockRestore(); + }); + + it("calls onViewportUpdate when the events list changes", () => { + // Complement of the previous test: a new events array must still + // refresh the virtual-scroll window, otherwise appended log lines + // would never become visible. + const spy = jest.spyOn(EventLogList.prototype, "onViewportUpdate"); + const { rerender } = render(); + spy.mockClear(); + + const moreEvents: EventLogItem[] = [ + ...mockEventList, + { id: "3", level: LogLevel.info, message: "baz" }, + ]; + rerender(); + expect(spy).toHaveBeenCalled(); + + spy.mockRestore(); + }); }); diff --git a/web/src/js/components/EventLog/EventList.tsx b/web/src/js/components/EventLog/EventList.tsx index d0b0e130d..c59a47bdb 100644 --- a/web/src/js/components/EventLog/EventList.tsx +++ b/web/src/js/components/EventLog/EventList.tsx @@ -49,14 +49,29 @@ export default class EventLogList extends Component< } componentDidUpdate( - _prevProps: EventLogListProps, + prevProps: EventLogListProps, _prevState: EventLogListState, snapshot: boolean, ) { if (snapshot) { autoscroll.adjustScrollTop(this.viewport); } - this.onViewportUpdate(); + // Only recompute the virtual-scroll window when the event list or the + // row height actually changed. Calling onViewportUpdate on every + // update (including the setState it produces itself) let + // setState -> componentDidUpdate -> setState spin without converging + // once measured row heights differed from the assumed rowHeight, + // surfacing as "Maximum update depth exceeded" while scrolling the + // event log. The same fix was applied to FlowTable in #8233. The + // other call sites still drive updates as needed: componentDidMount, + // the resize listener, the viewport onScroll, and setHeight when a + // row is first measured. + if ( + prevProps.events !== this.props.events || + prevProps.rowHeight !== this.props.rowHeight + ) { + this.onViewportUpdate(); + } } onViewportUpdate() {