fastify-error icon indicating copy to clipboard operation
fastify-error copied to clipboard

implement RFC7807

Open Uzlopak opened this issue 3 years ago • 4 comments

This is a PR for #64

This implements an RFC7807 conform toRFC7807 method to Fastify Error.

The only thing which is slightly deviating from RFC7807 is, that I dont put the attributes of the additional Error-Details on the root of the Error-Object. RFC7807 suggests, that if you have errors, you should put them into the root like this:

 {
  "type": "https://example.net/validation-error",
  "title": "Your request parameters didn't validate.",
  "invalid-params": [ {
                        "name": "age",
                        "reason": "must be a positive integer"
                      },
                      {
                        "name": "color",
                        "reason": "must be 'green', 'red' or 'blue'"}
                    ]
  }

I dont want that we have the necessity to enforce that some words are reserved, as we have e.g. already type and title in the root of the Object, so I implemented it by saying all additional errors should be in an details attribute.

But of course I could implement it like RFC7807 suggests, and put the attributes on the root of the Object, if you request it.

What we should then discuss is, if it is good or bad, that I implemented details as an Array. I also tend to implement the details as an Object.

Looking for your feedback

Uzlopak avatar Feb 22 '22 10:02 Uzlopak

On second thought, I think details should be an Object and not an Array.

Uzlopak avatar Feb 22 '22 11:02 Uzlopak

@mcollina

Do you prefer it more like this?

'use strict'

const { inherits, format } = require('util')

function createError (code, message, statusCode = 500, Base = Error, uriReference) {
  if (!code) throw new Error('Fastify error code must not be empty')
  if (!message) throw new Error('Fastify error message must not be empty')

  code = code.toUpperCase()

  function FastifyError (a, b, c, instance, details) {
    if (!new.target) {
      return new FastifyError(a, b, c, instance, details)
    }
    Error.captureStackTrace(this, FastifyError)
    this.name = 'FastifyError'
    this.code = code

    // more performant than spread (...) operator
    if (a && b && c) {
      this.message = format(message, a, b, c)
    } else if (a && b) {
      this.message = format(message, a, b)
    } else if (a) {
      this.message = format(message, a)
    } else {
      this.message = message
    }

    this.statusCode = statusCode || undefined
    this.uriReference = uriReference || undefined
    this.instance = instance || undefined
    this.details = details || undefined
  }
  FastifyError.prototype[Symbol.toStringTag] = 'Error'

  FastifyError.prototype.toString = function () {
    return `${this.name} [${this.code}]: ${this.message}`
  }

  FastifyError.prototype.toRFC7807 = function (instance, details) {
    return {
      type: this.uriReference || 'about:blank',
      title: this.name,
      status: this.statusCode,
      detail: this.message,
      instance: this.instance || instance || '',
      code: this.code,
      details: this.details || details || {}
    }
  }

  inherits(FastifyError, Base)

  return FastifyError
}

module.exports = createError

Uzlopak avatar Feb 22 '22 11:02 Uzlopak

I think this RFC is too tightly coupled to application implementations to be implemented generically.

jsumners avatar Feb 22 '22 14:02 jsumners

I think this RFC is too tightly coupled to application implementations to be implemented generically.

I'd say it's not good to be implemented generically by default indeed. On the other hand there is some aspects that are quite appealing for some of our use cases. Like the parsing of the AJV errors for bad requests.

Shouldn't this functionnality be another function? Like createRFCerror or something?

zekth avatar Feb 22 '22 15:02 zekth