Skip to content

Commit 22cfa41

Browse files
committed
bug: Fix multiple feedback file attachments
Ensure that the feedback form limits the user to 5 attachments and shows a warning toast if too many are added Ensure that multiple file attachments are supported in email, which requires buffering all the attachments as the email provider does not seem to support multiple file streams at once
1 parent f7445c6 commit 22cfa41

3 files changed

Lines changed: 26 additions & 25 deletions

File tree

apps/api/src/app/services/user-feedback.service.ts

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ENV, getExceptionLog, logger, rollbarServer } from '@jetstream/api-config';
22
import { sendUserFeedbackEmail } from '@jetstream/email';
33
import { Request } from 'express';
4-
import fs from 'node:fs';
4+
import fs from 'node:fs/promises';
55
import z from 'zod';
66

77
export const UserFeedbackPayloadSchema = z.object({
@@ -67,11 +67,18 @@ export async function handleUserFeedbackEmail(
6767
const files = (req.files as Express.Multer.File[] | undefined) || [];
6868

6969
// Prepare attachments from uploaded files
70-
const attachments = files.map((file, i) => ({
71-
data: fs.createReadStream(file.path),
72-
filename: filenames?.[i] || file.originalname,
73-
contentType: file.mimetype,
74-
}));
70+
// NOTE: ideally we would have the ability to stream the files to avoid loading them all into memory
71+
// but this is unsupported by the email service provider
72+
const attachments: { data: Buffer; filename: string; contentType: string }[] = [];
73+
let i = 0;
74+
for (const file of files) {
75+
attachments.push({
76+
data: await fs.readFile(file.path),
77+
filename: filenames?.[i] || file.originalname,
78+
contentType: file.mimetype,
79+
});
80+
i++;
81+
}
7582

7683
// Send email with feedback
7784
await sendUserFeedbackEmail(ENV.JETSTREAM_EMAIL_REPLY_TO, userId, feedbackContent, attachments);
@@ -96,21 +103,11 @@ export async function cleanupFeedbackAttachments(req: Request, loggerInfo: Recor
96103
return;
97104
}
98105
const filePaths = files.map((file) => file.path);
99-
await Promise.allSettled(
100-
filePaths.map((path) => {
101-
return new Promise<void>((resolve, reject) => {
102-
// Clean up temp files
103-
return fs.unlink(path, (err) => {
104-
if (err) {
105-
logger.error({ message: err.message }, 'Error deleting temp feedback attachment file');
106-
reject(err);
107-
return;
108-
}
109-
resolve();
110-
});
111-
});
112-
}),
113-
);
106+
// Clean up temp files
107+
const results = await Promise.allSettled(filePaths.map((path) => fs.unlink(path)));
108+
if (results.some((result) => result.status === 'rejected')) {
109+
throw new Error('Error deleting one or more temp feedback attachment files');
110+
}
114111
(req.log || logger).info({ filePaths, ...loggerInfo }, 'Temp feedback attachment files cleaned up');
115112
} catch (ex) {
116113
(req.log || logger).error({ ...getExceptionLog(ex), ...loggerInfo }, 'Error during cleanup of feedback attachments');

libs/shared/ui-core/src/app/ErrorBoundaryFallback.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,10 @@ export const ErrorBoundaryFallback: FunctionComponent<FallbackProps> = ({ error,
1414
if (error && rollbar) {
1515
try {
1616
logger.error(error);
17-
// logger.error(componentStack);
1817
rollbar.error(`[UNCAUGHT] ${error.message}`, error, {
1918
errorName: error.name,
2019
message: error.message,
2120
stack: error.stack,
22-
// componentStack,
2321
});
2422
} catch (ex) {
2523
try {
@@ -30,7 +28,7 @@ export const ErrorBoundaryFallback: FunctionComponent<FallbackProps> = ({ error,
3028
}
3129
}
3230
}
33-
}, [/** componentStack, */ error, rollbar]);
31+
}, [error, rollbar]);
3432

3533
function resetPage() {
3634
window.location.reload();

libs/ui/src/lib/user-feedback-widget/UserFeedbackWidget.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,13 @@ export const UserFeedbackWidget = () => {
9595
fireToast({ message: 'You can only upload up to 5 screenshots.', type: 'warning' });
9696
return;
9797
}
98-
setScreenshots((prev) => [...prev, fileContent]);
98+
setScreenshots((prev) => {
99+
if (prev.length >= 5) {
100+
setTimeout(() => fireToast({ message: 'You can only upload up to 5 screenshots.', type: 'warning' }), 0);
101+
return prev;
102+
}
103+
return [...prev, fileContent];
104+
});
99105
};
100106

101107
const handleRemoveScreenshot = (index: number) => {

0 commit comments

Comments
 (0)