langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-22 04:25:29 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

[PR #30257] fix: chat log message loading race conditions and infinite loops #32749

New Issue
Closed
opened 2026-02-21 20:52:00 -05:00 by yindo · 0 comments
yindo commented 2026-02-21 20:52:00 -05:00
Owner
Copy Link
Copy Source

Original Pull Request: https://github.com/langgenius/dify/pull/30257

State: closed
Merged: No

Summary

Chat message logs exhibit race conditions under slow network conditions: concurrent scroll events trigger duplicate requests with stale pagination state, causing messages to load out of order or duplicate. Additionally, two conflicting scroll listeners and ineffective throttling create infinite loading loops.

Changes

Request Queue Pattern

  • Added requestQueueRef, pendingRequestIdRef, lastScrollTopRef, scrollTimeoutRef to coordinate async operations
  • Each request receives unique ID; responses from superseded requests are discarded
  • Incoming requests wait for ongoing request completion, preventing concurrent fetches

Pagination State Management

  • Capture allChatItems at request initialization to avoid stale closures during async operations
  • Strict Set-based deduplication replaces retry-on-duplicate logic that caused infinite loops
  • Stop pagination when no new unique items returned

Unified Scroll Handler

  • Consolidate two conflicting scroll listeners into single debounced handler (200ms)
  • Only trigger on upward scroll within 50px of top
  • Passive event listener with proper cleanup

Code Reduction

  • Removed redundant loadMoreMessages function (134 lines)
  • Removed isLoading state in favor of isLoadingRef for synchronous checks
  • Net: -109 lines
// Before: Multiple listeners, stale closures
const loadMoreMessages = async () => {
  const answerItems = allChatItems.filter(...)  // Stale state
  // ... retry logic that can loop infinitely
}

// After: Single source of truth, request coordination
const fetchData = async () => {
  if (requestQueueRef.current) {
    await requestQueueRef.current
    return
  }
  const requestId = `${Date.now()}-${Math.random()}`
  const currentItems = allChatItems  // Captured at request time
  // ... discard if pendingRequestIdRef !== requestId
}

Screenshots

Not applicable - bug fix maintains existing UI behavior

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
Original prompt

Problem Description

When viewing chat message logs in the console with slow network conditions, the following bugs occur:

Issues

  1. Message Loading Order Chaos: Messages appear in incorrect chronological order when scrolling to load more history
  2. Infinite Loading Loop: Scrolling triggers continuous API requests, sometimes loading duplicate messages
  3. Race Conditions: Multiple concurrent requests caused by rapid scrolling lead to data inconsistency

Root Causes

After analyzing web/app/components/app/log/list.tsx, I identified several critical issues:

1. Race Condition in Loading State

const isLoadingRef = useRef(false)

const fetchData = useCallback(async () => {
  if (isLoadingRef.current)  // ❌ Not preventing concurrent requests effectively
    return
    
  try {
    isLoadingRef.current = true
    // ... async network request
  } finally {
    isLoadingRef.current = false
  }
}, [allChatItems, ...])

Problem: With slow networks and fast scrolling:

  • First request is still pending
  • User scrolls again before isLoadingRef.current is checked
  • Second request fires with outdated allChatItems state
  • Responses arrive in unpredictable order → message chaos

2. Incorrect Pagination Anchor Selection

const answerItems = allChatItems.filter(item => item.isAnswer)
const oldestAnswerItem = answerItems[answerItems.length - 1]  // ❌ May select wrong item during concurrent updates
if (oldestAnswerItem?.id)
  params.first_id = oldestAnswerItem.id

Problem:

  • When first request hasn't returned, allChatItems contains stale data
  • Second request uses same first_id, fetching duplicate data
  • Or after first request updates allChatItems, pagination skips messages

3. Multiple Conflicting Scroll Listeners

The code has TWO separate scroll listeners that can trigger simultaneously:

// Listener 1: Uses loadMoreMessages (around line 505-592)
useEffect(() => {
  const handleScroll = () => {
    if (isNearTop && hasMore && !isLoading) {
      loadMoreMessages()  // ❌ First loading function
    }
  }
  scrollContainer.addEventListener('scroll', handleScroll)
}, [hasMore, isLoading, loadMoreMessages])

// Listener 2: Uses fetchData (around line 617-656)
useEffect(() => {
  const handleScroll = () => {
    if (scrollTop < SCROLL_THRESHOLD_PX && hasMore && !isLoadingRef.current) {
      fetchData()  // ❌ Second loading function!
    }
  }
  scrollableDiv.addEventListener('scroll', handleScroll)
}, [threadChatItems.length, hasMore, fetchData])

Problem: Both listeners can fire simultaneously, causing duplicate API requests

4. Incomplete Throttle Mechanism

let lastLoadTime = 0  // ❌ Local variable, resets on every effect recreation
const throttleDelay = 200

const handleScroll = () => {
  const now = Date.now()
  if (isNearTop && hasMore && !isLoading && (now - lastLoadTime > throttleDelay)) {
    lastLoadTime = now
    loadMoreMessages()
  }
}

Problem: lastLoadTime is a local variable inside useEffect, so it resets whenever the effect re-runs

5. Retry Logic Causing Infinite Loops

// In loadMoreMessages function (around line 446-503)
const existingIds = new Set(allChatItems.map(item => item.id))
const uniqueNewItems = newItems.filter(item => !existingIds.has(item.id))

if (uniqueNewItems.length === 0) {
  // Retry with next item, but can cause infinite loops! ❌
  if (allChatItems.length > 1) {
    const nextId = allChatItems[1].id.replace('question-', '').replace('answer-', '')
    const retryParams = { ...params, first_id: nextId }
    const retryRes = await fetchChatMessages(...)
    // ... more retry logic
  }
}

Problem: This retry logic can trigger endless requests if server consistently returns duplicates

**Original Pull Request:** https://github.com/langgenius/dify/pull/30257 **State:** closed **Merged:** No --- ## Summary Chat message logs exhibit race conditions under slow network conditions: concurrent scroll events trigger duplicate requests with stale pagination state, causing messages to load out of order or duplicate. Additionally, two conflicting scroll listeners and ineffective throttling create infinite loading loops. ## Changes ### Request Queue Pattern - Added `requestQueueRef`, `pendingRequestIdRef`, `lastScrollTopRef`, `scrollTimeoutRef` to coordinate async operations - Each request receives unique ID; responses from superseded requests are discarded - Incoming requests wait for ongoing request completion, preventing concurrent fetches ### Pagination State Management - Capture `allChatItems` at request initialization to avoid stale closures during async operations - Strict Set-based deduplication replaces retry-on-duplicate logic that caused infinite loops - Stop pagination when no new unique items returned ### Unified Scroll Handler - Consolidate two conflicting scroll listeners into single debounced handler (200ms) - Only trigger on upward scroll within 50px of top - Passive event listener with proper cleanup ### Code Reduction - Removed redundant `loadMoreMessages` function (134 lines) - Removed `isLoading` state in favor of `isLoadingRef` for synchronous checks - Net: -109 lines ```typescript // Before: Multiple listeners, stale closures const loadMoreMessages = async () => { const answerItems = allChatItems.filter(...) // Stale state // ... retry logic that can loop infinitely } // After: Single source of truth, request coordination const fetchData = async () => { if (requestQueueRef.current) { await requestQueueRef.current return } const requestId = `${Date.now()}-${Math.random()}` const currentItems = allChatItems // Captured at request time // ... discard if pendingRequestIdRef !== requestId } ``` ## Screenshots Not applicable - bug fix maintains existing UI behavior ## Checklist - [ ] This change requires a documentation update, included: [Dify Document](https://github.com/langgenius/dify-docs) - [x] I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!) - [x] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change. - [x] I've updated the documentation accordingly. - [x] I ran `dev/reformat`(backend) and `cd web && npx lint-staged`(frontend) to appease the lint gods <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> ## Problem Description When viewing chat message logs in the console with slow network conditions, the following bugs occur: ### Issues 1. **Message Loading Order Chaos**: Messages appear in incorrect chronological order when scrolling to load more history 2. **Infinite Loading Loop**: Scrolling triggers continuous API requests, sometimes loading duplicate messages 3. **Race Conditions**: Multiple concurrent requests caused by rapid scrolling lead to data inconsistency ### Root Causes After analyzing `web/app/components/app/log/list.tsx`, I identified several critical issues: #### 1. **Race Condition in Loading State** ```tsx const isLoadingRef = useRef(false) const fetchData = useCallback(async () => { if (isLoadingRef.current) // ❌ Not preventing concurrent requests effectively return try { isLoadingRef.current = true // ... async network request } finally { isLoadingRef.current = false } }, [allChatItems, ...]) ``` **Problem**: With slow networks and fast scrolling: - First request is still pending - User scrolls again before `isLoadingRef.current` is checked - Second request fires with outdated `allChatItems` state - Responses arrive in unpredictable order → message chaos #### 2. **Incorrect Pagination Anchor Selection** ```tsx const answerItems = allChatItems.filter(item => item.isAnswer) const oldestAnswerItem = answerItems[answerItems.length - 1] // ❌ May select wrong item during concurrent updates if (oldestAnswerItem?.id) params.first_id = oldestAnswerItem.id ``` **Problem**: - When first request hasn't returned, `allChatItems` contains stale data - Second request uses same `first_id`, fetching duplicate data - Or after first request updates `allChatItems`, pagination skips messages #### 3. **Multiple Conflicting Scroll Listeners** The code has TWO separate scroll listeners that can trigger simultaneously: ```tsx // Listener 1: Uses loadMoreMessages (around line 505-592) useEffect(() => { const handleScroll = () => { if (isNearTop && hasMore && !isLoading) { loadMoreMessages() // ❌ First loading function } } scrollContainer.addEventListener('scroll', handleScroll) }, [hasMore, isLoading, loadMoreMessages]) // Listener 2: Uses fetchData (around line 617-656) useEffect(() => { const handleScroll = () => { if (scrollTop < SCROLL_THRESHOLD_PX && hasMore && !isLoadingRef.current) { fetchData() // ❌ Second loading function! } } scrollableDiv.addEventListener('scroll', handleScroll) }, [threadChatItems.length, hasMore, fetchData]) ``` **Problem**: Both listeners can fire simultaneously, causing duplicate API requests #### 4. **Incomplete Throttle Mechanism** ```tsx let lastLoadTime = 0 // ❌ Local variable, resets on every effect recreation const throttleDelay = 200 const handleScroll = () => { const now = Date.now() if (isNearTop && hasMore && !isLoading && (now - lastLoadTime > throttleDelay)) { lastLoadTime = now loadMoreMessages() } } ``` **Problem**: `lastLoadTime` is a local variable inside useEffect, so it resets whenever the effect re-runs #### 5. **Retry Logic Causing Infinite Loops** ```tsx // In loadMoreMessages function (around line 446-503) const existingIds = new Set(allChatItems.map(item => item.id)) const uniqueNewItems = newItems.filter(item => !existingIds.has(item.id)) if (uniqueNewItems.length === 0) { // Retry with next item, but can cause infinite loops! ❌ if (allChatItems.length > 1) { const nextId = allChatItems[1].id.replace('question-', '').replace('answer-', '') const retryParams = { ...params, first_id: nextId } const retryRes = await fetchChatMessages(...) // ... more retry logic } } ``` **Problem**: This retry logic can trigger endless requests if server consistently returns duplicates
yindo added the pull-request label 2026-02-21 20:52:00 -05:00
yindo closed this issue 2026-02-21 20:52:00 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#32749
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?