Skip to content

Commit bf607d3

Browse files
authored
fix: prevent awaiting suggestion search results in TextComposerMiddlewareExecutor (#1525)
## Goal Avoid potential UI problems caused by delays in delivering the search results
1 parent c93b035 commit bf607d3

File tree

2 files changed

+13
-29
lines changed

2 files changed

+13
-29
lines changed

src/messageComposer/middleware/textComposer/TextComposerMiddlewareExecutor.ts

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { createCommandsMiddleware } from './commands';
22
import { createMentionsMiddleware } from './mentions';
33
import { createTextComposerPreValidationMiddleware } from './validation';
4+
import { MiddlewareExecutor } from '../../../middleware';
45
import type {
56
ExecuteParams,
67
MiddlewareExecutionResult,
78
MiddlewareHandler,
89
} from '../../../middleware';
9-
import { MiddlewareExecutor } from '../../../middleware';
10-
import { withCancellation } from '../../../utils/concurrency';
1110
import type {
1211
Suggestion,
12+
Suggestions,
1313
TextComposerMiddlewareExecutorOptions,
1414
TextComposerState,
1515
} from './types';
@@ -58,28 +58,14 @@ export class TextComposerMiddlewareExecutor<
5858
initialValue: initialState,
5959
});
6060

61-
if (result && result.state.suggestions) {
62-
try {
63-
const searchResult = await withCancellation(
64-
'textComposer-suggestions-search',
65-
async () => {
66-
await result.state.suggestions?.searchSource.search(
67-
result.state.suggestions?.query,
68-
);
69-
},
70-
);
71-
if (searchResult === 'canceled') return { ...result, status: 'discard' };
72-
} catch (error) {
73-
// Clear suggestions on search error
74-
return {
75-
...result,
76-
state: {
77-
...result.state,
78-
suggestions: undefined,
79-
},
80-
};
81-
}
82-
}
61+
const { query, searchSource } = result.state.suggestions ?? ({} as Suggestions);
62+
/**
63+
* Catching error just for sanity purposes.
64+
* The BaseSearchSource.search() method returns debounced result.
65+
* That means the result of the previous search call as the debounced call result is unknown at the moment.
66+
* Custom search source implementation should handle errors meaningfully internally.
67+
*/
68+
searchSource?.search(query)?.catch(console.error);
8369

8470
return result;
8571
}

test/unit/MessageComposer/middleware/textComposer/TextComposerMiddlewareExecutor.test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,13 @@ describe('TextComposerMiddlewareExecutor', () => {
208208
expect(textComposer.suggestions).toBeUndefined();
209209
});
210210

211-
it('should handle search errors and cancellations', async () => {
211+
it('should not be impacted by errors triggered by search source query', async () => {
212212
const {
213213
channel,
214214
messageComposer: { textComposer },
215215
} = setup();
216216
const mockSearchSource = {
217-
search: vi.fn().mockImplementation(() => {
218-
throw new Error('Search failed');
219-
}),
217+
search: vi.fn().mockRejectedValue(new Error('Search failed')),
220218
activate: vi.fn(),
221219
resetinitialValue: vi.fn(),
222220
resetState: vi.fn(),
@@ -240,7 +238,7 @@ describe('TextComposerMiddlewareExecutor', () => {
240238
});
241239

242240
expect(mockSearchSource.search).toHaveBeenCalled();
243-
expect(result.state.suggestions).toBeUndefined();
241+
expect(result.state.suggestions).toBeDefined();
244242
});
245243

246244
describe('commands middleware', () => {

0 commit comments

Comments
 (0)