Make HttpResponseNotFound behaves like an Exception

[anonimusprogramus]

Hi Loic and everyone.

Scenario:

A GET/PUT/DELETE of /users/{id} needs a common function to check user existence, or else the process ends and returns a 404 Response.

user.controller.ts

export class UserController {
  @dependency
  private userService: UserService

  @Get('/:id')
  async getOneById(ctx: Context) {
    const user = await this.userService.getUserOrFail(ctx.request.params.id)
    return new HttpResponseOK(user)
  }
}

user.service.ts

async getUserOrFail(id) {
  const user = await User.findOne({ id: +id })

  if (!user) return new HttpResponseNotFound({ error: 'User Not Found' })

  return user
}

Result:

When user not found, it returns 200 Response with these object.

{
    "body": {
        "error": "User Not Found"
    },
    "isHttpResponse": true,
    "stream": false,
    "cookies": {},
    "headers": {},
    "isHttpResponseClientError": true,
    "isHttpResponseNotFound": true,
    "statusCode": 404,
    "statusMessage": "NOT FOUND"
}

Question:

How to make HttpResponseNotFound behaves like an Exception?

I mean, I want the process to just ends and returns 404 with my own error message, not returning back to controller.

Please advise, thank you.

[LoicPoullain]

One "hacky" way to do this is to throw a custom error and catch it in AppController.handleError.

For example:

import { renderError } from '@foal/core';

class NotFoundError extends Error {}

class AppController {

  @Get('/')
  hello() {
    throw new NotFoundError({ message: 'User not found' });
  }

  handleError(error: Error, ctx: Context): HttpResponse|Promise<HttpResponse> {
    if (error instanceof NotFoundError) {
      return new HttpResponseBadRequest(error.message);
    }

    return renderError(err, ctx);
  }

}

const app = createApp(AppController, {
  methods: {
    handleError: true
  }
})

There is one drawback to this approach, however. Post functions in hooks are not executed. This means that if you use @TokenRequired for example and that you changed the session, it won't be saved.

I leave the issue open and move it to Maybe / Pending / To think about. Maybe the framework will include a proper way to do this in the future. If others users are interested, feel free to add a 👍 on the issue.

[anonimusprogramus]

TIL the handleError is quite powerful.

I think this hacky way will do, thank you!

[LoicPoullain]

Proposal

Basic usage

The hooks, controllers and services can throw these errors: PermissionDenied, ValidationError and ObjectDoesNotExist.

These errors are caught in controller methods and hooks and are converted into HttpResponseForbidden, HttpResponseBadRequest and HttpResponseNotFound. The framework behaves as if the hooks or methods had returned these responses instead of raising an error.

This allows to throw permission, validation and not found errors through the services while keeping the HTTP logic in the controllers and hooks.

class TaskService {
  async read(id, user) {
    const task = Task.find({ id });
    if (task.user.id !== user.id) {
     throw new PermissionError();
    }
    return task;
  }
}

class Controller {
  @dependency
  tasks: TaskService;

  @Get('/tasks/:id')
  readTask(ctx) {
    return this.read(ctx.params.id, ctx.user);
  }

Custom handlers

It is possible to customize these conversions with the AppController methods handlePermissionDenied, handleValidationError, handleObjectDoesNotExist.

src/index.ts

const app = createApp(AppController, {
  methods: {
    handlePermissionDenied: true,
    // handleValidationError: true,
    // handleObjectDoesNotExist: true,
  }
});

app.controller.ts

class AppController {

  handlePermissionDenied(err) {
    if (Config.get('settings.debug', false)) {
      return new HttpResponseForbidden();
    }

    return new HttpResponseBadRequest();
  }

}

GraphQL

Managing errors with GraphQL is tedious in Foal. We could implement the same logic with the GraphQLController.

export class ApiController extends GraphQLController {
  schema = schema;
  resolvers = root;

  handlePermissionDenied(err) {
    // ...
  }
}

[kpyfan]

@LoicPoullain does your proposal here allow for running the return function on hooks? We've got a lot of logic that happens in our hooks (especially around sessions and cookies) and we're looking for a solution to get them to run the return portion of the hook even if we have to handle errors somehow. Any advice on how to do that?

[LoicPoullain]

@LoicPoullain does your proposal here allow for running the return function on hooks?

@kpyfan yes totally. The errors described above will be converted in HttpResponse directly for each hook/controller method unlike regular errors.

@Hook(() => (response) => console.log(2, response))
@Hook(() => { console.log(1); throw new PermissionDenied() })
handle() {
  console.log(3)
}

output

1
A HttpResponseForbidden
2

We've got a lot of logic that happens in our hooks (especially around sessions and cookies) and we're looking for a solution to get them to run the return portion of the hook even if we have to handle errors somehow. Any advice on how to do that?

Currently the solution what I see would be simply to add a try/catch exception in the hooks to convert them to HttpResponses.

[kpyfan]

@LoicPoullain not entirely sure how feasible this would be, but an extension to your proposal:

Rather than defining a few specific errors that are automatically converted (PermissionError, ObjectDoesNotExist, etc), can we use your proposed customization options (duplicated below for convenience) to provide a way for apps to globally handle errors?

const app = createApp(AppController, {
  methods: {
    handlePermissionDenied: true,
    // handleValidationError: true,
    // handleObjectDoesNotExist: true,
  }
});

Instead, we'd have something that maps a given error type to a handler function that would be run in that case.

function handlePermissionError(e: PermissionError) {
    console.log(e.getMessage);
    return new HttpResponseForbidden();
}

const app = createApp(AppController, {
  errorHandlers: {
    PermissionError: handlePermissionError,
  }
});

This allows apps to really customize how each error is handled, while still allowing for the run of post-functions on hooks. A good example of a usecase here is for customizing error messages and alerting. For example, if I have my app built in a way that all of my expected exceptions are caught at the controller level and handled, I'd like to have a global "Error Catcher" that would allow me to catch any unexpected errors, log out a statement for later investigation, and return a reasonable error message to the end user.

[LoicPoullain]

This allows apps to really customize how each error is handled, while still allowing for the run of post-functions on hooks. A good example of a usecase here is for customizing error messages and alerting. For example, if I have my app built in a way that all of my expected exceptions are caught at the controller level and handled, I'd like to have a global "Error Catcher" that would allow me to catch any unexpected errors, log out a statement for later investigation, and return a reasonable error message to the end user.

Thank your for sharing this.

Maybe the easier (and more maintainable) solution would be to simply allow the run of post-functions when an error is thrown. When an error is thrown in a controller or a hook, the framework will handle it and convert to an HttpResponse using the default exception handler or AppController.handleError. The controller or hook would work as if it had returned a response.

This is how Django and Laravel seems to manage their errors in middlewares:

• https://docs.djangoproject.com/en/3.0/topics/http/middleware/#exception-handling
• Can't catch exception in middleware laravel/framework#14573 (comment)

This has the advantage to keep things simpler without adding extra exceptions in the frameworks (PermissionDenied, etc) or optional configuration in createApp.

This is a breaking change however because post-functions will have to check if the response status code is 500 if they do not wish to be executed (ex: https://docs.djangoproject.com/fr/1.11/_modules/django/contrib/sessions/middleware/). This change can be added in version 2.

[LoicPoullain]

Resolved in v2