Closed nathanbrizzee-cdcr closed 2 years ago
claustres
commented
2 years ago Sorry we are not familiar with Koa and any help is welcome on this. It seems it does not like the way we register the middleware used for healthcheck, which is in express format: https://github.com/kalisio/feathers-distributed/blob/master/lib/index.js#L436. Maybe we should make this optional in config and/or detect if Koa is used (not sure if possible with Feathers).
jchamb
commented
2 years ago @claustres feathers-swaggers working branch for v5 has some test helpers. https://github.com/feathersjs-ecosystem/feathers-swagger/blob/custom-methods-v5/lib/helpers.js
claustres
commented
2 years ago Thanks @jchamb, by the way to you have any example on how to declare a similar route with Koa ?
jchamb
commented
2 years ago I'm not familiar with internals of this package at all. I actually just saw this as I was checking to see if ya'll had v5 support while digging around some options for micro service discovery.
If you are just asking how to declare routes in koa, koa doesn't have the get and other helpers like express unless you register https://github.com/koajs/router. I haven't had time to look at feathers koa adapter yet to see what they are doing either so don't know if thats part of the adapter middleware or not. You can do really basic route additions though doing something like the below.
app.use(async (ctx, next) => {
if (ctx.url === 'something') {
ctx.body = 'Sweet';
}
return next();
});feathers-swagger is basically just doing that, with their conditionals.
Note I don't maintain/contribute to feathers-swagger either. I just knew that existed as I was playing with v5 and testing our dependencies will upgrade smoothly. Maybe their core contributor Mairu would have better advice for you.
nathanbrizzee-cdcr
commented
2 years ago Thank you @claustres . I tried to figure out how to fix this problem but I kept getting hung up with the part that registers the middleware. I have not used koa before and since it is now the default for Feathers V5, I thought I'd give it a try. I could never get the healthcheck end point to work properly (in Feathers V4) and I started debugging that one as well.
I was wondering if @daffl might be able to take a look to see if there is an easy solution to getting this to work with Feathers V5 and koa since koa is now the default for the new generator and he's done a little V5 work in this project.
@jchamb , Thanks for the input on Feathers-swagger. We use that package as well and I haven't tried in with Feathers V5 yet. If you are looking for a great microservices discover package, THIS IS IT! We are using it in Production with docker swarm and it has been working great except for a few things that are turning out to be our not using it properly. If you have looked into using a service mesh of any kind with Kubernetes or a gateway like express-gateway, this package is far simpler and just works! Just make sure you use it either as a gateway, or to distribute multiple instances of the same app and not both at the same time (lesson learned) and be selective on which apps listen to which services in gateway mode. After hours and hours and hours of research, we have concluded that feathers-distributed is super simple to get working and just works and nothing else out there comes close. Excellent excellent work by @claustres!
nathanbrizzee-cdcr
commented
2 years ago @claustres, I have an update to my last comment. I saw your refactor check-in so I thought I would try it. I updated my package.json to this:
"@kalisio/feathers-distributed": "kalisio/feathers-distributed#master",I tried running the app again and this time it started up. Hurray!! So I created a second V5 application exactly the same but changed its startup port to 3031. I added a service to each application with different names and started both of them. They both PUB/SUB with each other. Another hurray! I tried calling a service in the first server via Postman on it's port number (3030). Hurray, It works. Then I tried calling the remote service in the second server (port 3031) but from the first server's port (3030). This one works too! So initial testing is showing that your code changes make feathers-distributed work with koa now. We just don't have the healthcheck api end point but that can come later. The good news is that there is a fix for Feathers V5 and koa. Thanks for such prompt work on this library. I can't wait for it to be released officially! It probably needs some more testing just to make sure.
daffl
commented
2 years ago I don't believe there is anything Express specific here other than the Healthcheck which I don't see a reason why it couldn't be a normal service like this:
const healthcheckRoute = (options.healthcheckPath || '/distribution/healthcheck/') + ':key'
debug('Initializing feathers-distributed healthcheck route', healthcheckRoute)
app.use(healthcheckRoute, {
async find (params) {
const key = params.route.key || 'default'
// Not yet registered
if (!app.serviceRequesters[key]) {
throw new NotFound(`No app registered with key ${key}`)
}
return healthcheck(app, key)
}
})Thanks @nathanbrizzee-cdcr for testing, great to know it works fine. I would like to make healthcheck available in Koa before releasing though.
Thanks @daffl for your proposal, as always relevant. I am not used to register a service when I only need a get route but now I realize I did it wrong because doing this will allow to support multiple middlewares. I will try to update this soon in the code, but making an additional test with koa will require more effort.
claustres
commented
2 years ago @nathanbrizzee-cdcr Could give it a try on the master bracnh ? The healthcheck should work with koa based on the suggestion from @daffl, Thanks.
nathanbrizzee-cdcr
commented
2 years ago Thanks so much @claustres . I can definitely test the healthcheck. Thanks to @daffl for the suggestion. That makes sense now. One other issue that I'm not sure how to handle is the express middleware errorHandler. I had to comment it out:
app.configure(
feathersDistribution({
hooks: {
before: {
all: [
// authenticate('jwt'),
]
}
},
services: (service) => true,
remoteServices: (service) => true,
middlewares: {
before: (req, res, next) => next(),
// after: express.errorHandler(), // With koa, this doesn't exist
},
key: "gateway",
})
)
Could we pass it in from the application since the app now imports errorHandler from either express or koa? Not sure how best to handle this.
This is from app.js from a V5 koa application. Express doesn't exist anywhere so I can't use the express.errorHandler()
import { koa, rest, bodyParser, errorHandler, parseAuthentication, cors } from '@feathersjs/koa'
...
// Set up Koa middleware
app.use(errorHandler())
...This is app.js from an V5 express application. I can use the express.errorHandler() middleware because express was imported into this app.
import express, { rest,json,urlencoded, cors, serveStatic, notFound, errorHandler } from '@feathersjs/express'
...
// Configure a middleware for 404s and the error handler
app.use(errorHandler({ logger }))Hi @claustres , I was able to test the distribution links. I guess I didn't understand how they worked before but I have a better understanding now. I had to change my package.json to the following to get your latest check-in
"@kalisio/feathers-distributed": "kalisio/feathers-distributed#80299d0a639fa4d32b287df051519b9ab5c4f2c3",When I call the distribution link for the remote server key thru the gateway address
localhost:3030/distribution/healthcheck/ms-1I get a list of services in the remote server.
{
"api/v1/anotherms-1": true,
"api/v1/ms-1": true
}And when I call the healthcheck end point for the gateway key from the remote server address, I get the services in the gatway
localhost:3031/distribution/healthcheck/gateway{
"api/v1/gs-1": true
}If I try calling the healthcheck api with the key of the server I am connecting to, I get a service not found. I guess that makes sense but I wasn't expecting it.
localhost:3030/distribution/healthcheck/gateway{
"name": "NotFound",
"message": "No app registered with key gateway",
"code": 404,
"className": "not-found"
}It would be really cool if I could call a distribution healthcheck link on a gateway and get a list of all services that have been registered without asking specifically for the key. That would help to see the status of all services in real time. Thanks so much for getting this code updated!!!
claustres
commented
2 years ago @nathanbrizzee-cdcr Good idea to add a global healthcheck endpoint now available using the app distribution key or without any key: https://github.com/kalisio/feathers-distributed/issues/109.
I think the Koa middleware could be registered similarly than the expresse one:
import { errorHandler } from '@feathersjs/koa'
feathersDistribution({
middlewares: {
after: errorHandler()
}
})Thanks so much @claustres . I tested the new healthcheck and it works great. I'll try your suggestion for the errorHandler. I don't know why I didn't think of it but it makes sense the way you showed it. Guess I was thinking too hard. :-)
Steps to reproduce
Create a new application with the Feathers 5 (currently 5.0.0-pre.31) generator and choose the defaults but specifically choose these -> (KOA) (JAVASCRIPT) Note: When I chose TypeScript, I couldn't get past the typescript errors so I went back and used JavaScript.
Add feathers-distributed according to the documentation page: app.js
Then try to run the application with
Expected behavior
The application should run like it does with ExpressJS
Actual behavior
Application crashes with the following output:
I've debugged through the feathers-distributed code to the error points, but I don't know how to fix them.
System configuration
Module versions (especially the part that's not working): package.json
NodeJS version: v16.15.1
NPM version: 8.11.0
Operating System: Ubuntu 22.04.1 LTS
Browser Version: n/a
React Native Version: n/a
Module Loader: n/a