refactor(response): stream file error callback can return body and status code

Author: thetutlage

package-lock.json

@@ -47,7 +47,7 @@
     "del-cli": "^3.0.0",
     "doctoc": "^1.4.0",
     "eslint": "^6.8.0",
-    "eslint-plugin-adonis": "^1.0.7",
+    "eslint-plugin-adonis": "^1.0.8",
     "fastify": "^2.12.0",
     "http-status-codes": "^1.4.0",
     "husky": "^4.2.3",

package.json

@@ -262,7 +262,10 @@ export class Response extends Macroable implements ResponseContract {
   /**
    * Stream the body to the response and handles cleaning up the stream
    */
-  private streamBody (body: ResponseStream, errorCallback?: ((error: NodeJS.ErrnoException) => any)) {
+  private streamBody (
+    body: ResponseStream,
+    errorCallback?: ((error: NodeJS.ErrnoException) => [string, number?]),
+  ) {
     return new Promise((resolve) => {
       let finished = false
 
@@ -280,7 +283,7 @@ export class Response extends Macroable implements ResponseContract {
         destroy(body)
 
         if (typeof (errorCallback) === 'function') {
-          this.endResponse(errorCallback(error))
+          this.endResponse(...errorCallback(error))
         } else {
           this.endResponse(
             error.code === 'ENOENT' ? 'File not found' : 'Cannot process file',
@@ -317,7 +320,7 @@ export class Response extends Macroable implements ResponseContract {
   private async streamFileForDownload (
     filePath: string,
     generateEtag: boolean,
-    errorCallback?: ((error: NodeJS.ErrnoException) => any),
+    errorCallback?: ((error: NodeJS.ErrnoException) => [string, number?]),
   ) {
     try {
       const stats = await statFn(filePath)
@@ -374,9 +377,12 @@ export class Response extends Macroable implements ResponseContract {
       return this.streamBody(createReadStream(filePath), errorCallback)
     } catch (error) {
       if (typeof (errorCallback) === 'function') {
-        this.endResponse(errorCallback(error))
+        this.endResponse(...errorCallback(error))
       } else {
-        this.endResponse('Cannot process file', 404)
+        this.endResponse(
+          error.code === 'ENOENT' ? 'File not found' : 'Cannot process file',
+          error.code === 'ENOENT' ? 404 : 500,
+        )
       }
     }
   }
@@ -726,7 +732,10 @@ export class Response extends Macroable implements ResponseContract {
    * }
    * ```
    */
-  public stream (body: ResponseStream, errorCallback?: ((error: NodeJS.ErrnoException) => any)): void {
+  public stream (
+    body: ResponseStream,
+    errorCallback?: ((error: NodeJS.ErrnoException) => [string, number?]),
+  ): void {
     if (typeof (body.pipe) !== 'function' || !body.readable || typeof (body.read) !== 'function') {
       throw new Error('response.stream accepts a readable stream only')
     }
@@ -765,7 +774,7 @@ export class Response extends Macroable implements ResponseContract {
   public download (
     filePath: string,
     generateEtag: boolean = this.config.etag,
-    errorCallback?: ((error: NodeJS.ErrnoException) => any),
+    errorCallback?: ((error: NodeJS.ErrnoException) => [string, number?]),
   ): void {
     this.lazyBody = {
       writer: this.streamFileForDownload,
@@ -784,7 +793,7 @@ export class Response extends Macroable implements ResponseContract {
     name?: string,
     disposition?: string,
     generateEtag?: boolean,
-    errorCallback?: ((error: NodeJS.ErrnoException) => any),
+    errorCallback?: ((error: NodeJS.ErrnoException) => [string, number?]),
   ) {
     name = name || filePath
     this.header('Content-Disposition', contentDisposition(name, { type: disposition }))

src/Response/index.ts

@@ -475,7 +475,7 @@ test.group('Response', (group) => {
         readStream.emit('error', new Error('Missing file'))
       }
 
-      response.stream(readStream, ({ message }) => message)
+      response.stream(readStream, ({ message }) => [message])
       response.finish()
     })
 
@@ -526,14 +526,16 @@ test.group('Response', (group) => {
   })
 
   test('write errors as response when downloading folder', async (assert) => {
+    await fs.ensureRoot()
+
     const server = createServer((req, res) => {
       const config = fakeConfig()
       const response = new Response(req, res, config)
       response.download(join(fs.basePath))
       response.finish()
     })
 
-    const { text } = await supertest(server).get('/').expect(404)
+    const { text } = await supertest(server).get('/').expect(500)
     assert.equal(text, 'Cannot process file')
   })
 
@@ -546,20 +548,22 @@ test.group('Response', (group) => {
     })
 
     const { text } = await supertest(server).get('/').expect(404)
-    assert.equal(text, 'Cannot process file')
+    assert.equal(text, 'File not found')
   })
 
-  test('raise errors as response when file is missing', async (assert) => {
+  test('return custom message and status when file is missing', async (assert) => {
     const server = createServer(async (req, res) => {
       const config = fakeConfig()
       const response = new Response(req, res, config)
 
-      response.download(join(fs.basePath, 'hello.html'), false)
+      response.download(join(fs.basePath, 'hello.html'), false, () => {
+        return ['Missing file', 400]
+      })
       response.finish()
     })
 
-    const { text } = await supertest(server).get('/').expect(404)
-    assert.equal(text, 'Cannot process file')
+    const { text } = await supertest(server).get('/').expect(400)
+    assert.equal(text, 'Missing file')
   })
 
   test('do not stream file on HEAD calls', async (assert) => {