Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bundled hooks customize errors #215

Closed
colinshen opened this Issue Jun 2, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@colinshen
Copy link

colinshen commented Jun 2, 2016

I can't find a way to do this in docs unless using my own middlewares.

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Jun 3, 2016

I've been thinking of adding afterError hooks to feathers-hooks that would allow to post-process errors at the service level. Something like

const errors = require('feathers-errors');

app.service('messages').afterError({
  create(hook) {
    const originalError = hook.error;

    hook.error = errors.Conflict(originalError.message);
  }
});

Would that solve your problem as well?

@colinshen

This comment has been minimized.

Copy link
Author

colinshen commented Jun 7, 2016

@daffl but for now. this is no way to do this. I will wait.
I think there are different hooks for a hook. For example, verifyToken and restrictToRoles. If we use the way you said, we have to find what error it is. So, I think would it be better to pass a string or json to the function?

create: [authHook.verifyToken('No token')],

or

create: [authHook.verifyToken({errorMessage:"No token"})],
@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Jun 8, 2016

@colinshen I like that second syntax option. I think we definitely need something more like an afterError hook or a special bundled error handling hook as that is the more scalable option but supporting custom error messages might do in the near term.

@ekryski ekryski added the Feature label Jun 8, 2016

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Jun 8, 2016

If we are considering customizing the error messages we should support it everywhere (e.g. the database adapters etc.).

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Jun 8, 2016

@daffl totally. Which is why I want to be lazy and not do that. 😝

In seriousness, I do think so. It is a lot of work but we need something to better handle errors in the hooks chain. Ideally I want something like the express error middleware, where you can chain together multiple error handling hooks to build up the error output you want.

So, if you wanted custom error messages you would check for the status code and modify as necessary.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Jun 30, 2016

So we had a revelation the other day. If you need custom conditions or custom errors you can simply wrap the hook in your own hook like so:

import { hooks } from 'feathers-authentication';

exports.hashPassword = function(options) {
  // Add any custom options

  function(hook) {
    return new Promise((resolve, reject) => {
      if (myCondition !== true) {
        return resolve(hook);
      }

      // call the original hook
      hooks.hashPassword(options)(hook)
        .then(hook => {
          // do something custom
          resolve(hook);
        })
        .catch(error => {
          // do any custom error handling
          error.message = 'my custom message';
          reject(error);
        });
    });
  });
}

Going to close this as this is the best solution. It's the most flexible and doesn't require any change to existing hooks.

@ekryski ekryski closed this Jun 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.