mac- / ratify

A Hapi plugin for validating the schema of path, query, request body, and response body params using JSON-schema
MIT License
71 stars 27 forks source link

Added support to report inner errors #15

Closed dschenkelman closed 10 years ago

dschenkelman commented 10 years ago

This PR adds support to report inner errors.

Nevertheless, I'm also thinking that the extract error message function should be a fallback to an optionally user defined function when configuring the plug-in.

Basically it would be used:

function customErrorMessageExtractor(error, defaultMessageExtractor){
  if (canExtractError(error)) { /* do so and return */ }
  return defaultMessageExtractor(error);
}

And also if no custom extractor is defined the default one would be used.

Thoughts?

mac- commented 10 years ago

PR looks good.

As far as your proposal on the custom method for extracting errors goes, I'm always for being able to provide flexibility. Is the use case just to be able to format the message differently? If so, maybe we can make it configurable such that we use a tokenized string to customize the errors. Something like this:

"Error: {message} - Description: {description} - {path}"

I don't know... I'm not pushing for one implementation over another, just throwing out ideas without even looking too deep into this :smile: I might have to sleep on this one.

dschenkelman commented 10 years ago

Tokenizing might be useful as well, but it is very different depending on the specific error. Missing required properties provide different info than formatting errors, etc.

We have a custom formatter for a small app we build which basically treats each of those fairly differently, not only in order.

Maybe being able to provide a tokenized string per error type and also have the ability to have a catch all tokenized string in case others fail?

dschenkelman commented 10 years ago

Hey @mac- have you given any thought to the formatting? Today I spent some time creating a simple formatter for a different purpose (using z-schema without ratify) and this is how it looks:

var extractors = {
  path: function (path){
    if (path){
      return path.substr(2).replace(/\//g, '.').replace(/\.\[/g, '[');
    }

    return '';
  },
  message: function(m){ return m; },
  description: function(d){ return '(' + d + ')'; },
  params: function extractParams(p) {
    if (Array.isArray(p)) {
      return extractParams(p[0]);
    }

    return p;
  }
}

function formatMessage(format, contextMessage, error){
  return ['path', 'description', 'message', 'params'].reduce(function(current, part){
    return current.replace('{' + part + '}', error[part] ? extractors[part](error[part]) : '');
  }, format).replace('{context}', contextMessage || '');
}

var FORMATS = {
  DEFAULT: '{context} \'{message}\' on property {path} {description}.',
  ENUM_MISMATCH: '{context} \'Invalid property "{params}"\' on property {path} {description}.'
};

function extractErrors(contextMessage, report) {
  return report.errors.map(function(error){
    return formatMessage(FORMATS[error.code] || FORMATS['DEFAULT'], contextMessage, error);
  }).join(',');
}

function createMessageReporter(contextMessage){
  return {
    extractMessage: extractErrors.bind(null, contextMessage)
  };
}

module.exports.init = createMessageReporter;

I'm thinking for ratify allowing users to override format and extractors would provide a good level of customization.

mac- commented 10 years ago

Yeah, I like what you are suggesting. I haven't had much time to play around with this idea. Do you think you'd want to create a PR for something like the solution you have above?

dschenkelman commented 10 years ago

Sure, I'll split this into phases.

  1. Create a separate module with code similar to the above to just handle the error reporting.
  2. Create a PR to have ratify use the module.
  3. Create a PR to allow users to extend how ratify formats errors.
dschenkelman commented 10 years ago

Closing in favor of #19. We can get back to this afterwards.