apps/meteor/app/emoji-custom/server/startup/emoji-custom.js (1)

101-107: Use file.contentType for the response Content-Type header instead of deriving it from the URL extension.

After the check at line 56 already validates that file.contentType.startsWith('image/'), the actual Content-Type header is set by re-deriving the type from the URL extension. If the stored contentType and the URL extension diverge (e.g., a file stored as emoji.svg with contentType: 'image/png'), the browser receives a Content-Type: image/svg+xml header while streaming PNG bytes, causing a render failure. custom-sounds.js correctly uses file.contentType directly at line 84—applying the same pattern here is simpler and eliminates the mismatch:

♻️ Proposed fix

-		if (/^svg$/i.test(extension)) {
-			res.setHeader('Content-Type', 'image/svg+xml');
-		} else if (/^png$/i.test(extension)) {
-			res.setHeader('Content-Type', 'image/png');
-		} else {
-			res.setHeader('Content-Type', 'image/jpeg');
-		}
+		res.setHeader('Content-Type', file.contentType);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-custom/server/startup/emoji-custom.js` around lines 101
- 107, The response Content-Type is being derived from the URL extension; change
it to use the stored MIME type instead by replacing the extension-based branch
with a single call that sets res.setHeader('Content-Type', file.contentType)
(the variable file and res are available in this scope and file.contentType was
already validated to start with "image/"); update the header-setting code in
emoji-custom.js where the extension checks occur to use file.contentType so the
streamed bytes match the declared MIME type.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

apps/meteor/app/emoji-custom/server/startup/emoji-custom.js (3)

104-110: Content-Type is still derived from the URL extension rather than file.contentType.

custom-sounds.js line 88 uses file.contentType directly from the stored record, which reflects the actual MIME type of the uploaded file. The emoji endpoint hardcodes a mapping (svg → image/svg+xml, png → image/png, otherwise image/jpeg) based purely on the URL extension. Given the PR's goal of backward-compatible content validation, using file.contentType here would also correctly handle emojis uploaded with non-standard extensions or no extension.

♻️ Proposed refactor

-		if (/^svg$/i.test(params.emoji.split('.').pop())) {
-			res.setHeader('Content-Type', 'image/svg+xml');
-		} else if (/^png$/i.test(params.emoji.split('.').pop())) {
-			res.setHeader('Content-Type', 'image/png');
-		} else {
-			res.setHeader('Content-Type', 'image/jpeg');
-		}
+		res.setHeader('Content-Type', file.contentType);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-custom/server/startup/emoji-custom.js` around lines 104
- 110, Replace the URL-extension based Content-Type logic in the emoji endpoint
with the stored file's MIME type: read and use the uploaded record's
file.contentType (the same property used in custom-sounds.js) to set
res.setHeader('Content-Type', file.contentType); if file.contentType is missing
or empty, fall back to a sensible default (e.g., 'image/jpeg') to preserve
compatibility; keep references to params.emoji only for lookup, not for deriving
the MIME type.

---

52-53: Consider replacing countByNameOrAlias with a lighter existence check for consistency.

countByNameOrAlias executes a count aggregation and returns a number, but the result is only used as a boolean (!emojiCount). The custom-sounds endpoint uses findOneById with { projection: { _id: 1 } }, which fetches a single minimal document and is a more idiomatic existence check. A parallel findOneByNameOrAlias (if available on the model) with the same minimal projection would be slightly more efficient and symmetric.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-custom/server/startup/emoji-custom.js` around lines 52
- 53, Replace the count-based existence check by using a minimal-document find:
replace the call to EmojiCustom.countByNameOrAlias(...) with
EmojiCustom.findOneByNameOrAlias(params.emoji.split('.')[0], { projection: {
_id: 1 } }) (or implement/find an equivalent finder), update the destructured
variable name from emojiCount to something like emojiDoc (or emojiExistsDoc),
and change the boolean check from !emojiCount to !emojiDoc to preserve behavior;
this mirrors the custom-sounds pattern and avoids a full count aggregation.

---

52-55: Wrap the Promise.all body in a try/catch to prevent response hangs on DB/IO errors.

EmojiCustom.countByNameOrAlias is a new DB operation. If it (or getFileWithReadStream) throws—e.g., on a MongoDB connection error—the rejected promise from the async handler is unhandled by WebApp.connectHandlers. Errors thrown inside async functions without a catch block result in unhandled promise rejections, which in Node.js 3.x+ can terminate the process, and in all versions leave the response socket open and the client hanging.

♻️ Proposed fix

 	return WebApp.connectHandlers.use('/emoji-custom/', async (req, res /* , next*/) => {
+		try {
 		const params = { emoji: decodeURIComponent(req.url.replace(/^\//, '').replace(/\?.*$/, '')) };
 		// ... rest of handler ...
 		file.readStream.pipe(res);
+		} catch (err) {
+			SystemLogger.error({ msg: 'Error serving emoji-custom', err });
+			if (!res.headersSent) {
+				res.writeHead(500);
+				res.end();
+			}
+		}
 	});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-custom/server/startup/emoji-custom.js` around lines 52
- 55, The Promise.all call that awaits EmojiCustom.countByNameOrAlias and
RocketChatFileEmojiCustomInstance.getFileWithReadStream must be wrapped in a
try/catch inside the WebApp.connectHandlers async handler to avoid unhandled
rejections; catch the error, log it (using the existing logger or
console.error), send an HTTP 500 response (res.writeHead(500) / res.end with a
short message) and ensure the response is closed so the client doesn't hang,
then return early. Locate the await Promise.all([...]) containing
EmojiCustom.countByNameOrAlias and
RocketChatFileEmojiCustomInstance.getFileWithReadStream and wrap it with this
error handling.

apps/meteor/app/custom-sounds/server/startup/custom-sounds.js (1)

51-54: Add a try/catch to handle DB/IO errors in the async handler.

CustomSounds.findOneById is a new async DB operation; if it (or getFileWithReadStream) throws, the unhandled rejection from the async middleware handler leaves the response hanging for the client.

♻️ Proposed fix

 	return WebApp.connectHandlers.use('/custom-sounds/', async (req, res /* , next*/) => {
+		try {
 		const fileId = decodeURIComponent(req.url.replace(/^\//, '').replace(/\?.*$/, ''));
 		// ... rest of handler ...
 		file.readStream.pipe(res);
+		} catch (err) {
+			SystemLogger.error({ msg: 'Error serving custom-sound', err });
+			if (!res.headersSent) {
+				res.writeHead(500);
+				res.end();
+			}
+		}
 	});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/custom-sounds/server/startup/custom-sounds.js` around lines
51 - 54, The Promise.all call invoking CustomSounds.findOneById and
RocketChatFileCustomSoundsInstance.getFileWithReadStream can throw and must be
wrapped in a try/catch inside the async request handler; add a try/catch around
the await Promise.all([CustomSounds.findOneById(...),
RocketChatFileCustomSoundsInstance.getFileWithReadStream(...)]) to log the error
(use the existing logger) and return a proper HTTP error response to the client
(e.g., send a 500 with a short message or call the handler's next(err)), so the
request does not hang when fileId DB/IO operations fail.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.

Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.

Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:49.806Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON which automatically deserializes Date objects. Stream handlers (e.g., sdk.stream()) receive Date fields as Date objects directly and do not require manual conversion using `new Date()`. Only REST API responses require manual date conversion because they return plain JSON where dates are serialized as strings.