fix: call abort() in useEffect cleanup to cancel WebDAV request on unmount

[Naetiksoni08]

Proposed changes (including videos or screenshots)

In SaveToWebdavModal.tsx, the useEffect cleanup was returning a reference to fileRequest.current?.abort instead of calling it:

  // Before (broken)
  useEffect(() => fileRequest.current?.abort, []);      

This meant that when the modal was closed mid-upload, the in-progress XMLHttpRequest was never cancelled — leaving a hanging network request and causing a memory leak.

// After (fixed)
 useEffect(() => () => fileRequest.current?.abort(), []);

Now when the modal unmounts, abort() is actually invoked, properly cancelling the WebDAV upload request.

Impact

• Cancels in-flight WebDAV requests when modal is closed mid-upload
• Prevents memory leaks from hanging XHR requests

Steps to test or reproduce

1. Open a room and click the WebDAV save option on any message attachment
2. Select a WebDAV account and start the file upload
3. Close the modal mid-upload
4. Before fix: request continues in background (visible in browser DevTools → Network tab)
5. After fix: request is immediately cancelled on modal close

Further comments

One-character-class fix — the cleanup arrow function was missing its invocation parentheses, so abort was never called.

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where file upload requests were not properly cancelled when closing the save dialog, ensuring better resource management.

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9e4348b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5edc80c2-1f4d-4f7a-9529-64e11443d5ca

📥 Commits

Reviewing files that changed from the base of the PR and between 24b3671 and 9e4348b.

📒 Files selected for processing (1)

• apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

• apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx

🧠 Learnings (6)

📓 Common learnings

Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:50.305Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.

Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 40075
File: apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx:69-71
Timestamp: 2026-04-10T22:42:05.539Z
Learning: In `apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx`, the submit handler converts an empty/whitespace-only description to `undefined` (`description?.trim() || undefined`) intentionally. All downstream image-rendering components (`AttachmentImage`, `ImagePreview`, `ImageItem`, `ImageGallery`) default `undefined` alt to `''`, so the `<img alt="">` attribute is always present. Do not flag this `undefined` conversion as a bug preventing alt text from being cleared.

📚 Learning: 2026-04-10T22:42:05.539Z

Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 40075
File: apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx:69-71
Timestamp: 2026-04-10T22:42:05.539Z
Learning: In `apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx`, the submit handler converts an empty/whitespace-only description to `undefined` (`description?.trim() || undefined`) intentionally. All downstream image-rendering components (`AttachmentImage`, `ImagePreview`, `ImageItem`, `ImageGallery`) default `undefined` alt to `''`, so the `<img alt="">` attribute is always present. Do not flag this `undefined` conversion as a bug preventing alt text from being cleared.

Applied to files:

• apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx

📚 Learning: 2026-04-17T18:33:24.670Z

Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:24.670Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.

Applied to files:

• apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx

📚 Learning: 2026-04-17T17:38:12.974Z

Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: packages/ui-kit/src/interactions/UserInteraction.ts:33-33
Timestamp: 2026-04-17T17:38:12.974Z
Learning: In RocketChat/Rocket.Chat (`packages/ui-kit/src/interactions/UserInteraction.ts`), `ViewSubmitUserInteraction` and `ViewClosedUserInteraction` intentionally do NOT include `rid` when the interaction originates from a **modal** surface. Modals are not scoped to a room, so no room id is available in that context. The `rid?: string` field is optional to support the contextual bar surface (where room context exists) while remaining absent for modals. Do not flag the absence of `rid` in modal viewSubmit/viewClosed interactions as a missing-context bug.

Applied to files:

• apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx

📚 Learning: 2026-04-18T12:32:50.305Z

Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:50.305Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.

Applied to files:

• apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx

📚 Learning: 2026-03-27T14:52:56.865Z

Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.

Applied to files:

• apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx

🔇 Additional comments (1)

apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx (1)

60-60: LGTM — cleanup now actually aborts the in-flight request.

Returning a cleanup callback that calls fileRequest.current?.abort() fixes the leak/hanging-request behavior while safely handling the “no request started” case.

---

Walkthrough

Fixed improper useEffect cleanup logic in SaveToWebdavModal component where XMLHttpRequest abort was referenced but not invoked. Now correctly returns a cleanup function that aborts any in-flight requests upon component unmount.

Changes

Cohort / File(s)	Summary
XMLHttpRequest Cleanup Fix apps/meteor/client/views/room/webdav/SaveToWebdavModal.tsx	Fixed useEffect cleanup function to properly abort in-flight XMLHttpRequest by returning a cleanup function instead of merely referencing the abort method without invocation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing the useEffect cleanup to properly call abort() on WebDAV requests during unmount.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Naetiksoni08]

Hey, just a gentle ping on this PR! 😊 Happy to address any feedback or make changes if needed.