fix(web): show topic test message send failures - #866
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR ensures Topic “test message” sends surface API failures to the user while keeping form-validation errors local, and adds a regression test for the failure case (Fixes #865).
Changes:
- Separates
sendForm.validateFields()from the send API call so validation failures don’t get treated as send failures. - Adds a user-visible error toast when
sendTopicMessage(...)rejects. - Adds a Vitest regression test asserting the error toast is shown and the modal remains open on send failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| web/src/pages/instance/topic.tsx | Refactors send handler to isolate validation and show an explicit error on API rejection. |
| web/src/pages/instance/tests/TopicPage.test.tsx | Adds coverage for the “send test message failed” path by asserting message.error is called. |
Suppressed comments (1)
web/src/pages/instance/topic.tsx:1
- The
catchblock swallows the actual error, which makes debugging/observability harder (no logging, no error details). Consider capturing the error (catch (err)) and at least logging it (or sending to your app’s telemetry) while still showing the user-friendly toast, so failures can be diagnosed in production.
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sendForm.resetFields(); | ||
| } catch { | ||
| // validation error, do nothing | ||
| message.error('发送测试消息失败，请稍后重试'); |
|
|
||
| it('shows an error when sending a test message fails', async () => { | ||
| const user = userEvent.setup(); | ||
| const errorSpy = vi.spyOn(message, 'error').mockImplementation(vi.fn()); |
| await waitFor(() => expect(topicServiceMocks.sendTopicMessage).toHaveBeenCalledTimes(1)); | ||
| expect(errorSpy).toHaveBeenCalledWith('发送测试消息失败，请稍后重试'); | ||
| expect(dialogTitle).toBeInTheDocument(); | ||
| errorSpy.mockRestore(); |
Signed-off-by: liuhy <liuhongyu@apache.org>
|
感谢贡献。发送测试消息失败时的错误提示已由主干的实例持久化改造（#951）覆盖：当前 Thanks for the contribution. Showing topic test-message send failures is already covered by the trunk's instance-persistence work (#951): |
Summary
sendTopicMessage(...)rejectsRelated Issue
Fixes #865
Tests
npm test -- --run src/pages/instance/__tests__/TopicPage.test.tsxnpm run lint -- src/pages/instance/topic.tsx src/pages/instance/__tests__/TopicPage.test.tsx(passes with existing fast-refresh warnings)