[platform-fastify] can't decompress responses with interceptors

kaioduarte commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

When using @fastify/compress plugin with the option global set to false and a custom interceptor, the response is broken, and the browser can't decode the data.

Interceptor repro:

@Injectable()
export class CompressInterceptor implements NestInterceptor {
  intercept(context: ExecutionContext, next: CallHandler) {
    return next.handle().pipe(
      tap((data) => {
        if (data) {
          const reply = context.switchToHttp().getResponse<FastifyReply>();
          reply.type('application/json').compress(data);
        }
      }),
    );
  }
}

These logs are printed to the console when that happens.

{
  "level": 30,
  "time": 1662403579167,
  "pid": 26651,
  "hostname": "Kaios-MBP.lan",
  "reqId": "req-4",
  "res": {
    "statusCode": 200
  },
  "msg": "stream closed prematurely"
}

{
  "level": 50,
  "time": 1662403579167,
  "pid": 26651,
  "hostname": "Kaios-MBP.lan",
  "reqId": "req-4",
  "err": {
    "type": "Error",
    "message": "premature close",
    "stack": "Error: premature close\n    at onclosenexttick (/Users/kaioduarte/dev/oss/nest-issue/app_name/node_modules/end-of-stream/index.js:54:86)\n    at processTicksAndRejections (node:internal/process/task_queues:78:11)"
  },
  "msg": "premature close"
}

Minimum reproduction code

https://github.com/kaioduarte/nest/tree/fix/fastify-compress/integration/compress

Steps to reproduce

No response

Expected behavior

It should respond with data that can be decompressed by the browser or API clients (Postman, Insomnia, etc.).

Package

[ ] I don't know. Or some 3rd-party package
[ ] code>@nestjs/common</code
[ ] code>@nestjs/core</code
[ ] code>@nestjs/microservices</code
[ ] code>@nestjs/platform-express</code
[X] code>@nestjs/platform-fastify</code
[ ] code>@nestjs/platform-socket.io</code
[ ] code>@nestjs/platform-ws</code
[ ] code>@nestjs/testing</code
[ ] code>@nestjs/websockets</code
[X] Other (see below)

Other package

@fastify/compress

NestJS version

9.0.11

Packages versions

[System Information]
OS Version     : macOS Monterey
NodeJS Version : v16.17.0
NPM Version    : 8.15.0 

[Nest CLI]
Nest CLI Version : 9.1.1 

[Nest Platform Information]
platform-fastify version : 9.0.11
schematics version       : 9.0.2
common version           : 9.0.11
core version             : 9.0.11
cli version              : 9.1.1

Node.js version

16.17.0

In which operating systems have you tested?

[X] macOS
[ ] Windows
[ ] Linux

Other

No response

jmcdo29 commented 2 years ago

Looks like res.compress() ends up calling res.send(). This isn't possible in an interceptor at all, whether it be express or fastify. Rather than using the compress on res you should probably use your own compression library implementation, or use the @fastify/compress as a middleware that compresses before response

kaioduarte commented 2 years ago

@jmcdo29, thanks for the prompt response!

I dug into this issue and found a workaround: to return the response object when the response is already compressed.

Source of the workaround: https://github.com/fastify/fastify-compress/blob/d9774965ed7029c5ea7f653c2c3503aaf9afa904/index.js#L224-L228

I have a fix with tests, let me know if you think it's not too hacky, so I can open the PR. 🙂

jmcdo29 commented 2 years ago

I'm not sure what the fix here would be? I see that compress checks if it should just all next, but not sure what that would mean in the reproduction you provided or what changes to Nest's codebase would be required

kaioduarte commented 2 years ago

@jmcdo29 you can check the changes needed on this commit:

https://github.com/kaioduarte/nest/commit/e27d02e512dd10d26cbbc9ca6487b646cf43a086

jmcdo29 commented 2 years ago

Okay, what about for express? We shouldn't be introducing divering functionality if we can help it.

In my opinion, @fastify/compress shouldn't be used in an interceptor like this unless you are able to get the raw compressed value back and return that with a map operator from rxjs. Sending a response from the interceptor goes against Nest's opinions, so I don't think we should follow through with it

kaioduarte commented 2 years ago

Sending a response from the interceptor goes against Nest's opinions, so I don't think we should follow through with it

Okay, so we can't proceed this way. I'll try to find another solution to compress per route only without a boilerplate, or use a fork.

Thanks anyway!

kaioduarte commented 2 years ago

This is the boilerplate-y way of doing this per route, just in case. If anyone has a better suggestion, I'm all ears!

import { Controller, Get, Res } from '@nestjs/common';
import { FastifyReply } from 'fastify';

@Controller()
export class AppController {
  @Get()
  getHello(@Res() reply: FastifyReply): unknown {
    const data = new Array(2048).fill(1);
    reply.type('application/json').compress(data);
    return reply;
  }
}

nestjs / nest