feat(dsb-client-gateway-api): file upload with chunk #70
Conversation
|
|
||
| let result: SendMessageResponseFile = null; | ||
| if (this.enableUploadChunk) { | ||
| const _file = await buffer(fs.createReadStream(filePath)); |
There was a problem hiding this comment.
it will cause high memory consumption as you read whole stream into a buffer - could cause buffer overflow
There was a problem hiding this comment.
yup, any suggestion. how encryption handle it ?
There was a problem hiding this comment.
streams
https://github.com/dannycho7/split-file-stream
you can use this library - split file to smaller chunks, upload them one by one. Calculate maxFileSize dynamically using some env. variable
| const _buffer = _file.slice(from, to); | ||
| //uploading file | ||
| result = | ||
| await this.ddhubFilesService.uploadFileChunk( |
There was a problem hiding this comment.
what if this fails? do you revert whole file?
There was a problem hiding this comment.
MB will handle it
| stopOnResponseCodes: ['10'], | ||
| } | ||
| ); | ||
| if (result.data == "") { |
There was a problem hiding this comment.
if promise is solved it means it was successful - we don't need this check
There was a problem hiding this comment.
MB only return 200 status
| ); | ||
| if (result.data == "") { | ||
| this.logger.log(`upload chunk ${currentChunkIndex} file with file name: ${originalname}`); | ||
| return result.data as SendMessageResponseFile; |
There was a problem hiding this comment.
why return empty string?
There was a problem hiding this comment.
that for continue loop chunk
There was a problem hiding this comment.
but it's incorrect - you return empty string without any data while telling this is SendMessageResponseFile type
| clientGatewayMessageId: string, | ||
| payloadEncryption: boolean, | ||
| transactionId?: string | ||
| ): Promise<SendMessageResponseFile> { |
|
|
||
| const _data = result.data as SendMessageResponseFile; | ||
| if (_data.recipients.failed > 0) { | ||
| throw new Error(`upload file with file name: ${originalname} failed`); |
There was a problem hiding this comment.
should we really throw an error if 1 recipient received message and second one didn't?
There was a problem hiding this comment.
that from existing
|
|
||
| const promise = () => |
There was a problem hiding this comment.
move it to separate method i.e. uploadChunks or smth like that
| @@ -76,6 +76,84 @@ export class DdhubFilesService extends DdhubBaseService { | |||
| } | |||
| ); | |||
|
|
|||
| const _data = result.data as SendMessageResponseFile; | |||
| if (_data.recipients.failed > 0) { | |||
| throw new Error(`upload file with file name: ${originalname} failed`); | |||
There was a problem hiding this comment.
add custom exception for it
| }); | ||
| await promise(); | ||
| } else { | ||
| //uploading file |
There was a problem hiding this comment.
remove comment, function name is obvious enough
azam-ismail
force-pushed
the
feat/large-upload
branch
from
b4c4a61 to
cd67743
Compare
|
|
nik-ewf
force-pushed
the
feat/large-upload
branch
from
b5fb9fc to
6031f1b
Compare
No description provided.