From f8f1a6873869df240a27033a071e0e5614d9ea28 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Wed, 7 May 2025 09:30:47 +0200 Subject: [PATCH] [web] Improve FileTooLargeError handling in FileWriter.js (#25278) * Improve FileTooLargeError handling in FileWriter.js * handle errors on passThrough stream * unlink files on error * fail `writeUrlToDisk` if content-length header is too large With Node 22, the test `Open In Overleaf - when POSTing a snip_uri for a file that is too large` fails. I initially tried replacing it with a check of the `content-length` header. But then I managed to make the old test pass by adding a handler (`passThrough.on('error', ...)`) * Unlink files asynchronously, add stream destroys on error * Remove eslint disables * Remove `stream.on('error', ...)` and `passThrough.on('error', ...)` * Revert `Content-Length` check * Re-add `stream.on('error', errorHandler)`; Remove it on 'response' * Only report unlink errors there is an error(!!) that's not ENOENT GitOrigin-RevId: fefe49519ec6f54df5eef69a2c2a75518f9d3748 --- .../web/app/src/infrastructure/FileWriter.js | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/services/web/app/src/infrastructure/FileWriter.js b/services/web/app/src/infrastructure/FileWriter.js index 2c98028f37..1a56f5fa26 100644 --- a/services/web/app/src/infrastructure/FileWriter.js +++ b/services/web/app/src/infrastructure/FileWriter.js @@ -1,9 +1,4 @@ -/* eslint-disable - n/handle-callback-err, - max-len, -*/ // TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. /* * decaffeinate suggestions: * DS102: Remove unnecessary code created because of implicit returns @@ -122,6 +117,16 @@ const FileWriter = { }).withCause(err || {}) } if (err) { + stream.destroy() + writeStream.destroy() + fs.unlink(fsPath, error => { + if (error && error.code !== 'ENOENT') { + logger.warn( + { error, fsPath }, + 'Failed to delete partial file after error' + ) + } + }) OError.tag( err, '[writeStreamToDisk] something went wrong writing the stream to disk', @@ -153,14 +158,16 @@ const FileWriter = { callback = _.once(callback) const stream = request.get(url) - stream.on('error', function (err) { + const errorHandler = function (err) { logger.warn( { err, identifier, url }, '[writeUrlToDisk] something went wrong with writing to disk' ) callback(err) - }) + } + stream.on('error', errorHandler) stream.on('response', function (response) { + stream.removeListener('error', errorHandler) if (response.statusCode >= 200 && response.statusCode < 300) { FileWriter.writeStreamToDisk(identifier, stream, options, callback) } else {