Non-JSON endpoints in a predominantly JSON environment

[egmacke]

I have a use case where the majority of my routes return JSON formatted data, so I've set up the Express adapter with useJson: true

Now I need to implement a few new routes which need to return plain text.

Other than using the injected response object (native types), is there any way to do this while using a HttpResponse as the return value? (In fact I'm trying to return either a 200 OK, or a 400 with a text body - currently using BadRequestHttpResponse but that gets serialised as JSON)

[notaphplover]

Hey @egmacke,

Other than using the injected response object (native types), is there any way to do this while using a HttpResponse as the return value? (In fact I'm trying to return either a 200 OK, or a 400 with a text body - currently using BadRequestHttpResponse but that gets serialised as JSON)

Non native types have some limitations. You can return a 200 text/plain response returning a string in the controller as docs. We actually let your adapter decide the mime type, I am almost positive express will set a text/plain in that case.

Returning a 400 via BadRequestHttpResponse leads to a JSON response, yeah. I guess you could probably force it using the SetHeader decorator, setting Content-Type to text/plain. I'm curious about your use case though.

[egmacke]

@notaphplover thanks, and yeah I see the OkHttpResponse works as expected for a plain string response.

For error responses though. The use case we have is that we're providing some callback endpoints for a third party service. That service expects error responses to come with a plain string response. Using @SetHeader solves the content-type issue, but not how it's serialised.

Would you be open to making a change to how the ErrorHttpResponse implementations get serialised?

Suggestion: Change the ErrorHttpResponse to take a body?: (object | string | number | boolean | Stream) | undefined like SuccessHttpReponse, then inject the error field if included. That way if only a string is passed in, it can be serialised as a plain string rather than getting wrapped as { message: 'string', error: undefined }

It feels like making the Success and Error HttpResponse implementations work in more similar/consistent ways would be better?

[notaphplover]

Hey @egmacke,

For error responses though. The use case we have is that we're providing some callback endpoints for a third party service. That service expects error responses to come with a plain string response. Using @SetHeader solves the content-type issue, but not how it's serialised.

I see, then it makes total sense to request that.

Suggestion: Change the ErrorHttpResponse to take a body?: (object | string | number | boolean | Stream) | undefined like SuccessHttpReponse, then inject the error field if included. That way if only a string is passed in, it can be serialised as a plain string rather than getting wrapped as { message: 'string', error: undefined }

I see, it makes sense. This would mean breaking ErrorHttpResponse behavior though if I'm not mistaken. I like the idea for the next major, I'm adding a task for it.

Meanwhile, let me suggest you a workaround:

import { Stream } from 'node:stream';

import { HttpStatusCode, HttpResponse, isHttpResponse } from '@inversifyjs/http-core';

const isHttpResponse: unique symbol = Symbol.for(
  '@inversifyjs/http-core/HttpResponse',
);
const isErrorHttpResponse: unique symbol = Symbol.for(
  '@inversifyjs/http-core/ErrorHttpResponse',
);

export class CustomErrorHttpResponse extends Error implements HttpResponse {
  public readonly [isHttpResponseSymbol]: true;
  public readonly [isErrorHttpResponse]: true;
  public readonly body?: object | string | number | boolean | Stream;

  constructor(
    public readonly statusCode: HttpStatusCode,
    body: object | string | number | boolean | Stream,
    errorOptions?: ErrorOptions,
  ) {
    super(message, errorOptions);

    this.body = body;
    this[isErrorHttpResponse] = true;
    this[isHttpResponseSymbol] = true;
  }

  public static is(value: unknown): value is CustomErrorHttpResponse {
    return (
      isHttpResponse(value) &&
      (value as Partial<ErrorHttpResponse>)[isErrorHttpResponse] === true
    );
  }
}

This way we trick the adapter to believe CustomErrorHttpResponse is an ErrorHttpResponse. This way you will be able to use CustomErrorHttpResponse to return 400 error responses with string bodies, leading to a plain/text content type. I know this workaround is crafty, I'll release the next major soon sice we stil don't have too many users using the library

[egmacke]

Needed a few minor tweaks. but otherwise that works perfectly. And it removes the need for calling @SetHeader. Thanks!

import { Stream } from 'node:stream';
import {
  ErrorHttpResponse,
  HttpStatusCode,
  isHttpResponse
} from '@inversifyjs/http-core';

const isHttpResponseSymbol: unique symbol = Symbol.for(
  '@inversifyjs/http-core/HttpResponse'
);
const isErrorHttpResponseSymbol: unique symbol = Symbol.for(
  '@inversifyjs/http-core/ErrorHttpResponse'
);

export class CustomErrorHttpResponse extends ErrorHttpResponse {
  public readonly [isHttpResponseSymbol]: true;
  public readonly [isErrorHttpResponseSymbol]: true;

  constructor(
    public override readonly statusCode: HttpStatusCode,
    public override readonly body?:
      | object
      | string
      | number
      | boolean
      | Stream
      | undefined,
    errorOptions?: ErrorOptions
  ) {
    super(statusCode, body?.toString() ?? '', undefined, errorOptions);

    this.body = body;
    this[isErrorHttpResponseSymbol] = true;
    this[isHttpResponseSymbol] = true;
  }

  public static override is(value: unknown): value is CustomErrorHttpResponse {
    return (
      isHttpResponse(value) &&
      (value as Partial<CustomErrorHttpResponse>)[isErrorHttpResponseSymbol] === true
    );
  }
}