Skip to content

Commit e2ddd2b

Browse files
ryansavaraRyan Savara
andcommitted
Fix S3 Uploading (#133)
Co-authored-by: Ryan Savara <ryan.savara@vatusa.net>
1 parent 35261a1 commit e2ddd2b

7 files changed

Lines changed: 154 additions & 80 deletions

File tree

src/app.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,14 @@ app.use((err: any, _req: Request, res: Response, next: NextFunction) => {
129129
return next(err);
130130
}
131131

132+
if (err.status && typeof err.status !== 'number') {
133+
err.status = 500;
134+
}
135+
136+
if (err.code && typeof err.code !== 'number') {
137+
err.code = 500;
138+
}
139+
132140
res.status(err.status || err.code || 500).json({
133141
message: err.message || 'An internal server error occurred.',
134142
});

src/controllers/event.ts

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { captureException } from '@sentry/node';
22
import { Router, type NextFunction, type Request, type Response } from 'express';
33
import { fileTypeFromFile } from 'file-type';
4-
import fs from 'fs/promises';
4+
import * as fs from 'fs';
55
import multer from 'multer';
66
import { sendMail } from '../helpers/mailer.js';
77
import { deleteFromS3, uploadToS3 } from '../helpers/s3.js';
@@ -26,6 +26,9 @@ const upload = multer({
2626
cb(null, `${Date.now()}-${file.originalname}`);
2727
},
2828
}),
29+
limits: {
30+
fileSize: 30 * 1024 * 1024, // 30MiB
31+
},
2932
});
3033

3134
router.get('/', async (_req: Request, res: Response, next: NextFunction) => {
@@ -817,20 +820,32 @@ router.post(
817820
message: 'Banner type not supported',
818821
};
819822
}
820-
if (req.file.size > 30 * 10240 * 10240) {
821-
// 10MiB
823+
824+
const filePath = req.file.path;
825+
let fileStream: fs.ReadStream | undefined;
826+
827+
try {
828+
fileStream = fs.createReadStream(filePath);
829+
830+
await uploadToS3(`events/${req.file.filename}`, fileStream, req.file.mimetype, {
831+
ContentDisposition: 'inline',
832+
});
833+
} catch (e) {
834+
captureException(e);
835+
822836
throw {
823-
code: status.BAD_REQUEST,
824-
message: 'Banner too large',
837+
code: 500,
838+
message: 'Error streaming file to storage',
825839
};
840+
} finally {
841+
try {
842+
fileStream?.close();
843+
fs.unlinkSync(filePath);
844+
} catch (_err) {
845+
// Do nothing, we don't care about this error
846+
}
826847
}
827848

828-
const tmpFile = await fs.readFile(req.file.path);
829-
830-
await uploadToS3(`events/${req.file.filename}`, tmpFile, req.file.mimetype, {
831-
ContentDisposition: 'inline',
832-
});
833-
834849
await EventModel.create({
835850
name: req.body.name,
836851
description: req.body.description,
@@ -957,24 +972,35 @@ router.put(
957972
message: 'File type not supported',
958973
};
959974
}
960-
if (req.file.size > 30 * 10240 * 10240) {
961-
// 30MiB
962-
throw {
963-
code: status.BAD_REQUEST,
964-
message: 'File too large',
965-
};
966-
}
967975

968-
// 🚨 **Delete Old Banner from S3**
969976
if (eventData.bannerUrl) {
970977
deleteFromS3(`events/${eventData.bannerUrl}`);
971978
}
972979

973-
const tmpFile = await fs.readFile(req.file.path);
980+
const filePath = req.file.path;
981+
let fileStream: fs.ReadStream | undefined;
974982

975-
await uploadToS3(`events/${req.file.filename}`, tmpFile, req.file.mimetype, {
976-
ContentDisposition: 'inline',
977-
});
983+
try {
984+
fileStream = fs.createReadStream(filePath);
985+
986+
await uploadToS3(`events/${req.file.filename}`, fileStream, req.file.mimetype, {
987+
ContentDisposition: 'inline',
988+
});
989+
} catch (e) {
990+
captureException(e);
991+
992+
throw {
993+
code: status.INTERNAL_SERVER_ERROR,
994+
message: 'Error streaming file to storage',
995+
};
996+
} finally {
997+
try {
998+
fileStream?.close();
999+
fs.unlinkSync(filePath);
1000+
} catch (_err) {
1001+
// Do nothing, we don't care about this error
1002+
}
1003+
}
9781004

9791005
eventData.bannerUrl = req.file.filename;
9801006
}

src/controllers/file.ts

Lines changed: 88 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { captureException } from '@sentry/node';
22
import { Router, type NextFunction, type Request, type Response } from 'express';
3-
import fs from 'fs/promises';
3+
import * as fs from 'fs';
44
import multer from 'multer';
55
import { deleteFromS3, uploadToS3 } from '../helpers/s3.js';
66
import { hasRole } from '../middleware/auth.js';
@@ -21,6 +21,9 @@ const upload = multer({
2121
cb(null, `${Date.now()}-${file.originalname}`);
2222
},
2323
}),
24+
limits: {
25+
fileSize: 250 * 1024 * 1024, // 250MiB
26+
},
2427
});
2528

2629
//#region Downloads
@@ -80,16 +83,28 @@ router.post(
8083
};
8184
}
8285

83-
if (req.file.size > 100 * 1024 * 1024) {
84-
// 100MiB
86+
const filePath = req.file.path;
87+
let fileStream: fs.ReadStream | undefined;
88+
89+
try {
90+
fileStream = fs.createReadStream(filePath);
91+
92+
await uploadToS3(`downloads/${req.file.filename}`, fileStream, req.file.mimetype);
93+
} catch (e) {
94+
captureException(e);
95+
8596
throw {
86-
code: status.BAD_REQUEST,
87-
message: 'File too large',
97+
code: 500,
98+
message: 'Error streaming file to storage',
8899
};
100+
} finally {
101+
try {
102+
fileStream?.close();
103+
fs.unlinkSync(filePath);
104+
} catch (_err) {
105+
// Do nothing, we don't care about this error
106+
}
89107
}
90-
const tmpFile = await fs.readFile(req.file.path);
91-
92-
await uploadToS3(`downloads/${req.file.filename}`, tmpFile, req.file.mimetype);
93108

94109
await DownloadModel.create({
95110
name: req.body.name,
@@ -128,37 +143,47 @@ router.put(
128143
}
129144

130145
if (!req.file) {
131-
// ✅ No updated file, just update metadata
132146
await DownloadModel.findByIdAndUpdate(req.params['id'], {
133147
name: req.body.name,
134148
description: req.body.description,
135149
category: req.body.category,
136150
}).exec();
137151
} else {
138-
// ✅ File size check (100MiB limit)
139-
if (req.file.size > 100 * 1024 * 1024) {
140-
throw { code: status.BAD_REQUEST, message: 'File too large' };
141-
}
142-
143-
// 🚨 **Step 1: Delete Old File from S3 (if it exists)**
144152
if (download.fileName) {
145153
deleteFromS3(`downloads/${download.fileName}`);
146154
}
147155

148-
// 🚀 **Step 2: Upload New File to S3**
149-
const tmpFile = await fs.readFile(req.file.path);
150-
await uploadToS3(`downloads/${req.file.filename}`, tmpFile, req.file.mimetype);
156+
const filePath = req.file.path;
157+
let fileStream: fs.ReadStream | undefined;
158+
159+
try {
160+
fileStream = fs.createReadStream(filePath);
161+
162+
await uploadToS3(`downloads/${req.file.filename}`, fileStream, req.file.mimetype);
163+
} catch (e) {
164+
captureException(e);
165+
166+
throw {
167+
code: 500,
168+
message: 'Error streaming file to storage',
169+
};
170+
} finally {
171+
try {
172+
fileStream?.close();
173+
fs.unlinkSync(filePath);
174+
} catch (_err) {
175+
// Do nothing, we don't care about this error
176+
}
177+
}
151178

152-
// ✅ **Step 3: Update Database with New File Name**
153179
await DownloadModel.findByIdAndUpdate(req.params['id'], {
154180
name: req.body.name,
155181
description: req.body.description,
156182
category: req.body.category,
157-
fileName: req.file.filename, // ✅ Save the new file reference
183+
fileName: req.file.filename,
158184
}).exec();
159185
}
160186

161-
// ✅ Log the update in dossier
162187
await DossierModel.create({
163188
by: req.user.cid,
164189
affected: -1,
@@ -181,21 +206,17 @@ router.delete(
181206
hasRole(['atm', 'datm', 'ta', 'fe', 'wm']),
182207
async (req: Request, res: Response, next: NextFunction) => {
183208
try {
184-
// 🚀 **Step 1: Fetch the file info from the database**
185209
const download = await DownloadModel.findById(req.params['id']).lean().exec();
186210
if (!download) {
187211
return res.status(status.NOT_FOUND).json({ error: 'File not found' });
188212
}
189213

190-
// 🗑️ **Step 2: Delete the file from S3 if it exists**
191214
if (download.fileName) {
192215
await deleteFromS3(`downloads/${download.fileName}`);
193216
}
194217

195-
// ❌ **Step 3: Delete the database entry**
196218
await DownloadModel.findByIdAndDelete(req.params['id']).exec();
197219

198-
// ✅ Log deletion in dossier
199220
await DossierModel.create({
200221
by: req.user.cid,
201222
affected: -1,
@@ -289,18 +310,30 @@ router.post(
289310
if (!req.file) {
290311
throw { code: status.BAD_REQUEST, message: 'File required' };
291312
}
292-
if (req.file.size > 100 * 1024 * 1024) {
293-
// 100MiB
313+
314+
const filePath = req.file.path;
315+
let fileStream: fs.ReadStream | undefined;
316+
317+
try {
318+
fileStream = fs.createReadStream(filePath);
319+
320+
await uploadToS3(`documents/${req.file.filename}`, fileStream, req.file.mimetype);
321+
} catch (e) {
322+
captureException(e);
323+
294324
throw {
295-
code: status.BAD_REQUEST,
296-
message: 'File too large',
325+
code: 500,
326+
message: 'Error streaming file to storage',
297327
};
328+
} finally {
329+
try {
330+
fileStream?.close();
331+
fs.unlinkSync(filePath);
332+
} catch (_err) {
333+
// Do nothing, we don't care about this error
334+
}
298335
}
299336

300-
const tmpFile = await fs.readFile(req.file.path);
301-
302-
await uploadToS3(`documents/${req.file.filename}`, tmpFile, req.file.mimetype);
303-
304337
await DocumentModel.create({
305338
name,
306339
category,
@@ -376,7 +409,6 @@ router.put(
376409
await document.save();
377410
} else {
378411
if (!req.file) {
379-
// ✅ No new file, just update metadata
380412
await DocumentModel.findOneAndUpdate(
381413
{ slug: req.params['slug'] },
382414
{
@@ -387,21 +419,33 @@ router.put(
387419
},
388420
).exec();
389421
} else {
390-
// ✅ File size check (100MiB limit)
391-
if (req.file.size > 100 * 1024 * 1024) {
392-
throw { code: status.BAD_REQUEST, message: 'File too large.' };
393-
}
394-
395-
// 🚨 **Step 1: Delete Old File from S3 (if it exists)**
396422
if (document.fileName) {
397423
await deleteFromS3(`documents/${document.fileName}`);
398424
}
399425

400-
// 🚀 **Step 2: Upload New File to S3**
401-
const tmpFile = await fs.readFile(req.file.path);
402-
await uploadToS3(`documents/${req.file.filename}`, tmpFile, req.file.mimetype);
426+
const filePath = req.file.path;
427+
let fileStream: fs.ReadStream | undefined;
428+
429+
try {
430+
fileStream = fs.createReadStream(filePath);
431+
432+
await uploadToS3(`documents/${req.file.filename}`, fileStream, req.file.mimetype);
433+
} catch (e) {
434+
captureException(e);
435+
436+
throw {
437+
code: 500,
438+
message: 'Error streaming file to storage',
439+
};
440+
} finally {
441+
try {
442+
fileStream?.close();
443+
fs.unlinkSync(filePath);
444+
} catch (_err) {
445+
// Do nothing, we don't care about this error
446+
}
447+
}
403448

404-
// ✅ **Step 3: Update Database with New File Name**
405449
await DocumentModel.findOneAndUpdate(
406450
{ slug: req.params['slug'] },
407451
{
@@ -415,7 +459,6 @@ router.put(
415459
}
416460
}
417461

418-
// ✅ Log update in dossier
419462
await DossierModel.create({
420463
by: req.user.cid,
421464
affected: -1,
@@ -438,7 +481,6 @@ router.delete(
438481
hasRole(['atm', 'datm', 'ta', 'fe', 'wm']),
439482
async (req: Request, res: Response, next: NextFunction) => {
440483
try {
441-
// 🚀 **Step 1: Fetch the document from the database**
442484
const doc = await DocumentModel.findById(req.params['id']).lean().exec();
443485
if (!doc) {
444486
throw {
@@ -447,15 +489,12 @@ router.delete(
447489
};
448490
}
449491

450-
// 🗑️ **Step 2: Delete the file from S3 if it exists**
451492
if (doc.fileName) {
452493
deleteFromS3(`documents/${doc.fileName}`);
453494
}
454495

455-
// ❌ **Step 3: Delete the database entry**
456496
await DocumentModel.findByIdAndDelete(req.params['id']).exec();
457497

458-
// ✅ Log deletion in dossier
459498
await DossierModel.create({
460499
by: req.user.cid,
461500
affected: -1,

0 commit comments

Comments
 (0)