Fix race condition in TUI code for handling async queries

If someone types in "l" and then "s" to search for "ls", then this will dispatch two async queries. If the query for "l" finishes after the query for "ls", then this will lead to the results for "l" getting incorrectly displayed. In practice, this is quite rare for human typing speeds so I had never noticed this. But, it causes an issue for test flakes and is the root cause of many of my recent changes around test flakes. Fixing this should improve test reliability significantly.
This commit is contained in:
David Dworken 2023-10-24 22:52:52 -07:00
parent bb96164ea8
commit e50f4d164b
No known key found for this signature in database

View File

@ -183,11 +183,18 @@ type bannerMsg struct {
banner string banner string
} }
type asyncQueryFinishedMsg struct { type asyncQueryFinishedMsg struct {
rows []table.Row // The query that finished running. Used to ensure that we only process this message if it matches the current query.
entries []*data.HistoryEntry searchedQuery string
searchErr error // The table rows and entries
forceUpdateTable bool rows []table.Row
maintainCursor bool entries []*data.HistoryEntry
// An error from searching, if one occurred
searchErr error
// Whether to force a full refresh of the table
forceUpdateTable bool
// Whether to maintain the cursor position
maintainCursor bool
// An updated search query. May be used for initial queries when they're invalid.
overriddenSearchQuery *string overriddenSearchQuery *string
} }
@ -270,7 +277,7 @@ func runQueryAndUpdateTable(m model, forceUpdateTable, maintainCursor bool) tea.
} }
return func() tea.Msg { return func() tea.Msg {
rows, entries, searchErr := getRows(m.ctx, hctx.GetConf(m.ctx).DisplayedColumns, query, PADDED_NUM_ENTRIES) rows, entries, searchErr := getRows(m.ctx, hctx.GetConf(m.ctx).DisplayedColumns, query, PADDED_NUM_ENTRIES)
return asyncQueryFinishedMsg{rows, entries, searchErr, forceUpdateTable, maintainCursor, nil} return asyncQueryFinishedMsg{query, rows, entries, searchErr, forceUpdateTable, maintainCursor, nil}
} }
} }
return nil return nil
@ -342,9 +349,11 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.isLoading = false m.isLoading = false
return m, nil return m, nil
case asyncQueryFinishedMsg: case asyncQueryFinishedMsg:
m = updateTable(m, msg.rows, msg.entries, msg.searchErr, msg.forceUpdateTable, msg.maintainCursor) if m.runQuery != nil && *m.runQuery == msg.searchedQuery {
if msg.overriddenSearchQuery != nil { m = updateTable(m, msg.rows, msg.entries, msg.searchErr, msg.forceUpdateTable, msg.maintainCursor)
m.queryInput.SetValue(*msg.overriddenSearchQuery) if msg.overriddenSearchQuery != nil {
m.queryInput.SetValue(*msg.overriddenSearchQuery)
}
} }
return m, nil return m, nil
default: default:
@ -686,12 +695,12 @@ func TuiQuery(ctx context.Context, initialQuery string) error {
go func() { go func() {
rows, entries, err := getRows(ctx, hctx.GetConf(ctx).DisplayedColumns, initialQuery, PADDED_NUM_ENTRIES) rows, entries, err := getRows(ctx, hctx.GetConf(ctx).DisplayedColumns, initialQuery, PADDED_NUM_ENTRIES)
if err == nil || initialQuery == "" { if err == nil || initialQuery == "" {
p.Send(asyncQueryFinishedMsg{rows: rows, entries: entries, searchErr: err, forceUpdateTable: true, maintainCursor: false, overriddenSearchQuery: nil}) p.Send(asyncQueryFinishedMsg{searchedQuery: initialQuery, rows: rows, entries: entries, searchErr: err, forceUpdateTable: true, maintainCursor: false, overriddenSearchQuery: nil})
} else { } else {
// initialQuery is likely invalid in some way, let's just drop it // initialQuery is likely invalid in some way, let's just drop it
emptyQuery := "" emptyQuery := ""
rows, entries, err := getRows(ctx, hctx.GetConf(ctx).DisplayedColumns, emptyQuery, PADDED_NUM_ENTRIES) rows, entries, err := getRows(ctx, hctx.GetConf(ctx).DisplayedColumns, emptyQuery, PADDED_NUM_ENTRIES)
p.Send(asyncQueryFinishedMsg{rows: rows, entries: entries, searchErr: err, forceUpdateTable: true, maintainCursor: false, overriddenSearchQuery: &emptyQuery}) p.Send(asyncQueryFinishedMsg{searchedQuery: emptyQuery, rows: rows, entries: entries, searchErr: err, forceUpdateTable: true, maintainCursor: false, overriddenSearchQuery: &emptyQuery})
} }
}() }()
// Async: Retrieve additional entries from the backend // Async: Retrieve additional entries from the backend