Pergunta

So, in the process of creating a user there are 4 possible outcomes:

  1. Username is already taken

  2. Email is already taken

  3. Username is invalid
  4. Email is invalid

Here is what I have in the controller for now:

  const user = await this.addUserService.add(username, email, password)

  if (user) {
    return {
      statusCode: 201,
      body: user
    }
  } else {
    return {
      statusCode: 400,
      body: // ???
    }
  }

The thing is, what should the addUserService return if the username/email is invalid or in use?

I am thinking about creating different error types, such as InvalidParamsError, ParamInUseError and share them between the UserController and UserService.

So I would have something like this:

  const result = await this.addUserService.add(username, email, password)

  if (result instanceof InvalidParamsError || result instanceof ParamInUseError) {
    return {
      statusCode: 401,
      body: result.message
    }
  } else { // then we assume user has been created
    return {
      statusCode: 201,
      body: 'User created!'
    }
  }

Is it okay to share these Error classes between the controller and the service? If not, or even if it's okay, is there a better way to handle errors when the outcome is not a simple true or false?

Thanks.

EDIT: Also, I just realized that the UserRepository checks if a username and/or email already exists. So I would have to share the ParamInUseError between three modules: UserController, UserService and UserRepository

Foi útil?

Solução

Returning multiple types (subtypes) is a completely valid approach, in order to represent ok/error behaviour of a method. For statically typed languages, constructs like union type could be used, in JavaScript this is easier, since you can simply return any object from a method.

Instead of returning a success/error objects from a method, it's also possible to simply throw from an underlying layer, and let the exception bubble up to a place in your app where it's appropriately handled. This is however sometimes frowned upon, as it may lead to abusing exception to a point where they act as a control flow and basically a modern goto.

How much you want to propagate between layers depends entirely up to you and your architecture. Consider a system which would consist of two modules:

  1. Money Account - allows you to make DEPOSIT, WITHDRAWAL and PURCHASE transactions,
  2. Ticket - allows you to buy a ticket for money from your money account.

Now, when making a WITHDRAWAL or PURCHASE transaction, the money account module may return an error: INSUFFICIENT_FUNDS.

The process of purchasing a ticket consists of creating a valid ticket, which is preceded by successfully processing a PURCHASE transaction (i.e. purchasing a ticket may end due to the INSUFFICIENT_FUNDS error).

In this case you you have basically these two options:

  1. fail the ticket purchase with the INSUFFICIENT_FUNDS error,
  2. fail the ticket purchase with a new error, e.g. PURCHASE_TRANSACTION_FAILED, where you could/should also append an original error, which would point to INSUFFICIENT_FUNDS.

That way each module has its own set of granular errors, while still preserving the original error situation.

Outras dicas

According to R. Martin's "Clean Architecture" everything must have a single responsibility, meaning it has only one reason to change. This is strongly correlated with another principle of clean coding and software engineering - separation of concerns.

Therefore if we were to follow these principles, your presentation layer of your domain has to have a separate unit, isolated from other ones, which has one purpose - handle errors.

Licenciado em: CC-BY-SA com atribuição
scroll top