diff options
author | Chocobozzz <me@florianbigard.com> | 2021-09-09 09:31:50 +0200 |
---|---|---|
committer | Chocobozzz <me@florianbigard.com> | 2021-09-09 09:35:30 +0200 |
commit | 790c2837ddcb443c0f1ea6adcdcb101dfe159d01 (patch) | |
tree | 2f1c957363e1c00475a7ab7c75c09efe61fba004 | |
parent | b4c945f3c727d1ce5de5a4af69d6dfa14c83468e (diff) | |
download | PeerTube-790c2837ddcb443c0f1ea6adcdcb101dfe159d01.tar.gz PeerTube-790c2837ddcb443c0f1ea6adcdcb101dfe159d01.tar.zst PeerTube-790c2837ddcb443c0f1ea6adcdcb101dfe159d01.zip |
Fix silent 500 after resumable upload
-rw-r--r-- | server/controllers/api/videos/upload.ts | 5 | ||||
-rw-r--r-- | server/helpers/upload.ts | 9 | ||||
-rw-r--r-- | server/tests/api/videos/resumable-upload.ts | 1 | ||||
-rw-r--r-- | shared/extra-utils/requests/requests.ts | 8 | ||||
-rw-r--r-- | shared/extra-utils/shared/abstract-command.ts | 16 | ||||
-rw-r--r-- | shared/extra-utils/videos/videos-command.ts | 15 |
6 files changed, 39 insertions, 15 deletions
diff --git a/server/controllers/api/videos/upload.ts b/server/controllers/api/videos/upload.ts index 7e87df8b1..55cb9cf20 100644 --- a/server/controllers/api/videos/upload.ts +++ b/server/controllers/api/videos/upload.ts | |||
@@ -2,7 +2,7 @@ import express from 'express' | |||
2 | import { move } from 'fs-extra' | 2 | import { move } from 'fs-extra' |
3 | import { basename } from 'path' | 3 | import { basename } from 'path' |
4 | import { getLowercaseExtension } from '@server/helpers/core-utils' | 4 | import { getLowercaseExtension } from '@server/helpers/core-utils' |
5 | import { deleteResumableUploadMetaFile, getResumableUploadPath } from '@server/helpers/upload' | 5 | import { getResumableUploadPath } from '@server/helpers/upload' |
6 | import { uuidToShort } from '@server/helpers/uuid' | 6 | import { uuidToShort } from '@server/helpers/uuid' |
7 | import { createTorrentAndSetInfoHash } from '@server/helpers/webtorrent' | 7 | import { createTorrentAndSetInfoHash } from '@server/helpers/webtorrent' |
8 | import { getLocalVideoActivityPubUrl } from '@server/lib/activitypub/url' | 8 | import { getLocalVideoActivityPubUrl } from '@server/lib/activitypub/url' |
@@ -130,9 +130,6 @@ export async function addVideoResumable (_req: express.Request, res: express.Res | |||
130 | const videoInfo = videoPhysicalFile.metadata | 130 | const videoInfo = videoPhysicalFile.metadata |
131 | const files = { previewfile: videoInfo.previewfile } | 131 | const files = { previewfile: videoInfo.previewfile } |
132 | 132 | ||
133 | // Don't need the meta file anymore | ||
134 | await deleteResumableUploadMetaFile(videoPhysicalFile.path) | ||
135 | |||
136 | return addVideo({ res, videoPhysicalFile, videoInfo, files }) | 133 | return addVideo({ res, videoPhysicalFile, videoInfo, files }) |
137 | } | 134 | } |
138 | 135 | ||
diff --git a/server/helpers/upload.ts b/server/helpers/upload.ts index 030a6b7d5..3cb17edd0 100644 --- a/server/helpers/upload.ts +++ b/server/helpers/upload.ts | |||
@@ -1,5 +1,3 @@ | |||
1 | import { METAFILE_EXTNAME } from '@uploadx/core' | ||
2 | import { remove } from 'fs-extra' | ||
3 | import { join } from 'path' | 1 | import { join } from 'path' |
4 | import { RESUMABLE_UPLOAD_DIRECTORY } from '../initializers/constants' | 2 | import { RESUMABLE_UPLOAD_DIRECTORY } from '../initializers/constants' |
5 | 3 | ||
@@ -9,13 +7,8 @@ function getResumableUploadPath (filename?: string) { | |||
9 | return RESUMABLE_UPLOAD_DIRECTORY | 7 | return RESUMABLE_UPLOAD_DIRECTORY |
10 | } | 8 | } |
11 | 9 | ||
12 | function deleteResumableUploadMetaFile (filepath: string) { | ||
13 | return remove(filepath + METAFILE_EXTNAME) | ||
14 | } | ||
15 | |||
16 | // --------------------------------------------------------------------------- | 10 | // --------------------------------------------------------------------------- |
17 | 11 | ||
18 | export { | 12 | export { |
19 | getResumableUploadPath, | 13 | getResumableUploadPath |
20 | deleteResumableUploadMetaFile | ||
21 | } | 14 | } |
diff --git a/server/tests/api/videos/resumable-upload.ts b/server/tests/api/videos/resumable-upload.ts index 857859fd3..59970aa94 100644 --- a/server/tests/api/videos/resumable-upload.ts +++ b/server/tests/api/videos/resumable-upload.ts | |||
@@ -113,6 +113,7 @@ describe('Test resumable upload', function () { | |||
113 | it('Should correctly delete files after an upload', async function () { | 113 | it('Should correctly delete files after an upload', async function () { |
114 | const uploadId = await prepareUpload() | 114 | const uploadId = await prepareUpload() |
115 | await sendChunks({ pathUploadId: uploadId }) | 115 | await sendChunks({ pathUploadId: uploadId }) |
116 | await server.videos.endResumableUpload({ pathUploadId: uploadId }) | ||
116 | 117 | ||
117 | expect(await countResumableUploads()).to.equal(0) | 118 | expect(await countResumableUploads()).to.equal(0) |
118 | }) | 119 | }) |
diff --git a/shared/extra-utils/requests/requests.ts b/shared/extra-utils/requests/requests.ts index 5bbf7f3bf..501e0b374 100644 --- a/shared/extra-utils/requests/requests.ts +++ b/shared/extra-utils/requests/requests.ts | |||
@@ -57,9 +57,15 @@ function makeActivityPubGetRequest (url: string, path: string, expectedStatus = | |||
57 | }) | 57 | }) |
58 | } | 58 | } |
59 | 59 | ||
60 | function makeDeleteRequest (options: CommonRequestParams) { | 60 | function makeDeleteRequest (options: CommonRequestParams & { |
61 | query?: any | ||
62 | rawQuery?: string | ||
63 | }) { | ||
61 | const req = request(options.url).delete(options.path) | 64 | const req = request(options.url).delete(options.path) |
62 | 65 | ||
66 | if (options.query) req.query(options.query) | ||
67 | if (options.rawQuery) req.query(options.rawQuery) | ||
68 | |||
63 | return buildRequest(req, { accept: 'application/json', expectedStatus: HttpStatusCode.BAD_REQUEST_400, ...options }) | 69 | return buildRequest(req, { accept: 'application/json', expectedStatus: HttpStatusCode.BAD_REQUEST_400, ...options }) |
64 | } | 70 | } |
65 | 71 | ||
diff --git a/shared/extra-utils/shared/abstract-command.ts b/shared/extra-utils/shared/abstract-command.ts index 021045e49..a57c857fc 100644 --- a/shared/extra-utils/shared/abstract-command.ts +++ b/shared/extra-utils/shared/abstract-command.ts | |||
@@ -40,6 +40,11 @@ interface InternalGetCommandOptions extends InternalCommonCommandOptions { | |||
40 | query?: { [ id: string ]: any } | 40 | query?: { [ id: string ]: any } |
41 | } | 41 | } |
42 | 42 | ||
43 | interface InternalDeleteCommandOptions extends InternalCommonCommandOptions { | ||
44 | query?: { [ id: string ]: any } | ||
45 | rawQuery?: string | ||
46 | } | ||
47 | |||
43 | abstract class AbstractCommand { | 48 | abstract class AbstractCommand { |
44 | 49 | ||
45 | constructor ( | 50 | constructor ( |
@@ -82,8 +87,15 @@ abstract class AbstractCommand { | |||
82 | }) | 87 | }) |
83 | } | 88 | } |
84 | 89 | ||
85 | protected deleteRequest (options: InternalCommonCommandOptions) { | 90 | protected deleteRequest (options: InternalDeleteCommandOptions) { |
86 | return makeDeleteRequest(this.buildCommonRequestOptions(options)) | 91 | const { query, rawQuery } = options |
92 | |||
93 | return makeDeleteRequest({ | ||
94 | ...this.buildCommonRequestOptions(options), | ||
95 | |||
96 | query, | ||
97 | rawQuery | ||
98 | }) | ||
87 | } | 99 | } |
88 | 100 | ||
89 | protected putBodyRequest (options: InternalCommonCommandOptions & { | 101 | protected putBodyRequest (options: InternalCommonCommandOptions & { |
diff --git a/shared/extra-utils/videos/videos-command.ts b/shared/extra-utils/videos/videos-command.ts index d35339c8d..63980c147 100644 --- a/shared/extra-utils/videos/videos-command.ts +++ b/shared/extra-utils/videos/videos-command.ts | |||
@@ -449,6 +449,8 @@ export class VideosCommand extends AbstractCommand { | |||
449 | 449 | ||
450 | const result = await this.sendResumableChunks({ ...options, pathUploadId, videoFilePath, size }) | 450 | const result = await this.sendResumableChunks({ ...options, pathUploadId, videoFilePath, size }) |
451 | 451 | ||
452 | await this.endResumableUpload({ ...options, pathUploadId }) | ||
453 | |||
452 | return result.body?.video || result.body as any | 454 | return result.body?.video || result.body as any |
453 | } | 455 | } |
454 | 456 | ||
@@ -542,6 +544,19 @@ export class VideosCommand extends AbstractCommand { | |||
542 | }) | 544 | }) |
543 | } | 545 | } |
544 | 546 | ||
547 | endResumableUpload (options: OverrideCommandOptions & { | ||
548 | pathUploadId: string | ||
549 | }) { | ||
550 | return this.deleteRequest({ | ||
551 | ...options, | ||
552 | |||
553 | path: '/api/v1/videos/upload-resumable', | ||
554 | rawQuery: options.pathUploadId, | ||
555 | implicitToken: true, | ||
556 | defaultExpectedStatus: HttpStatusCode.NO_CONTENT_204 | ||
557 | }) | ||
558 | } | ||
559 | |||
545 | quickUpload (options: OverrideCommandOptions & { | 560 | quickUpload (options: OverrideCommandOptions & { |
546 | name: string | 561 | name: string |
547 | nsfw?: boolean | 562 | nsfw?: boolean |