TypeError: Cannot read properties of null (reading 'toString') in getColoredStatusCode at return statusCode.toString();

[sfodor]

Version of FoalTS: v4.2.0 Environment: Windows local development env. Logger format: 'dev'

This is the second time when I've got "TypeError: Cannot read properties of null" error coming from the node_modules\@foal\core\lib\common\logging\logger.utils.js:44:23 and return statusCode.toString(); .

[sfodor]

It was happening again:

[LoicPoullain]

Hi @sfodor 👋

Thank you reporting this issue. I also got the error several times and finally figured out when it comes from.

Steps to reproduce

1. Use this request handler:

   export class ApiController {
   @Get("/")
   async index(ctx: Context) {
   await new Promise((resolve) => setTimeout(resolve, 3000));
   return new HttpResponseOK("Hello world!");
   }
   }

2. Then run the command curl http://localhost:3001/api and then terminate the process before the request has been completed.

The error shows up at this moment, when the request/response cycle completes before a response was sent to the client, because there is no status code or response time in this case.

Reference 1: https://github.com/expressjs/morgan/issues/242#issuecomment-666424995 Reference 2: https://github.com/expressjs/morgan#status

It probably happens, in development, when a tab is closed or when the client server hotly reloads.

Impact

This error is only raised when using the logger dev format so it should not appear in any production environment.

Solution

I'll make the change so that the frameworks logs the following line in this case:

[12:32:26 PM] INFO GET /api null - null ms

[LoicPoullain]

Resolved in v4.3.0 👍

[sfodor]

Not sure if statusCode === unknow is ever possible, but would it be safer to avoid using toString() in the end of the getColoredStatusCode function? How about the following or something similar?

function getColoredStatusCode(statusCode: number | null): string {
  if (statusCode >= 500) {
    return `\u001b[31m${statusCode}\u001b[39m`;
  }
  if (statusCode >= 400) {
    return `\u001b[33m${statusCode}\u001b[39m`;
  }
  if (statusCode >= 300) {
    return `\u001b[36m${statusCode}\u001b[39m`;
  }
  if (statusCode >= 200) {
    return `\u001b[32m${statusCode}\u001b[39m`;
  }
  return `\u001b[31m${statusCode}\u001b[39m`;
}

[LoicPoullain]

Good point @sfodor 👍 I changed the code as suggested to use a template literal. Thus, with the null and undefined values which don't have a toString(), the values will see will be rendered to strings.

The first line if (statusCode >= 500) { will not compile though if the value can be null. This is why I kept the first if (statusCode === null) { condition in the newly opened PR.