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

Does not work with not found error handler #3

Closed
claustres opened this issue Sep 21, 2017 · 5 comments
Closed

Does not work with not found error handler #3

claustres opened this issue Sep 21, 2017 · 5 comments
Labels

Comments

@claustres
Copy link
Member

claustres commented Sep 21, 2017

For now Feathers does not allow to register new services after the app has been setup due to the fact the error handler will be hit first.

The current workaround is to not register any not found error handler.

@Flofie
Copy link

Flofie commented Mar 27, 2018

Seems to be working with v3.

@claustres
Copy link
Member Author

Would be good news ! Do you mean that if we remove this comment https://github.com/kalisio/feathers-distributed/blob/master/example/gateway/src/app.js#L60 it works either with REST or WebSockets ?

@Flofie
Copy link

Flofie commented Mar 27, 2018

exactly. But I only ran your test suite and added notFound and errorHandler to the app:

function createApp (index) {
    let app = express(feathers());
    app.configure(socketio());
    app.configure(express.rest());
    app.configure(authentication({ secret: '1234' }));
    let strategies = ['jwt'];
    app.configure(jwt());
    app.use(express.notFound());
    app.use(express.errorHandler());
    if (index === gateway) {
      strategies.push('local');
      app.configure(local());
    }
    // The `authentication` service is used to create a JWT.
    // The before `create` hook registers strategies that can be used
    // to create a new valid JWT (e.g. local or oauth2)
    app.service('authentication').hooks({
      before: {
        create: [authentication.hooks.authenticate(strategies)],
        remove: [authentication.hooks.authenticate('jwt')]
      }
    });

But with these changes the tests still complete perfectly.

@Flofie
Copy link

Flofie commented Dec 13, 2018

If you activate the default express.notFound() handler of feathers this handler blocks the request to the remote services. You alway get a 404.
You could just remove the notFound Handler but the problem with that is that you then get a 404 and a html page returned. Since my client only accepts json we had a problem.

Here is my solution:

const Url = require('url-parse');

// Configure a middleware for 404s and the error handler
app.use((req, res, next) => {
  const url = new Url(req.path);
  if (app._router.stack.find(r => r.route && r.route.path && r.route.path === url.pathname)) {
    next();
  } else {
    const { url } = req;
    const message = `Page not found${process.env.NODE_ENV !== 'production' ? ': ' + url : ''}`;
    next(new NotFound(message, { url }));
  }
});
app.use(express.errorHandler({ logger }));

Hope that helps.

@claustres claustres changed the title Does not work with error handler Does not work with not found error handler Jun 6, 2019
@claustres
Copy link
Member Author

We can now close this I think as there is no generic solution. If you exactly know in your app the available remote services you can wait on startup until they are alive then register the handler, see eg our tests https://github.com/kalisio/feathers-distributed/blob/master/test/index.test.js#L170. However, if you want to dynamically create new services after that you might face the issue so you should not register the handler at all and rely only on timeouts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants