Просмотр исходного кода

[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
Antoine Clausse 1 год назад
Родитель
Сommit
07b37abcb3
1 измененных файлов с 14 добавлено и 7 удалено
  1. 14 7
      services/web/app/src/infrastructure/FileWriter.js

+ 14 - 7
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 {