robtweed / qewd Goto Github PK

Lines 43 to 45 in 6b9b898

 if (!appModule.handlers) appModule.handlers = {}; 

 if (appModule.handlers) this.handlers[application] = appModule.handlers;

if (!appModule.handlers) appModule.handlers = {}; 
  
this.handlers[application] = appModule.handlers;

because if (appModule.handlers) on line 45 is always true

fs.exists is deprecated

https://nodejs.org/api/fs.html#fs_fs_exists_path_callback

Lines 63 to 66 in 71a0457

 
 fs.exists(fragPath, function(exists) { 

 if (exists) { 

 fs.readFile(fragPath, 'utf8', function(err, data) {

Maybe pass err as reason to finished?

Lines 66 to 76 in 71a0457

 fs.readFile(fragPath, 'utf8', function(err, data) { 

 if (!err) { 

 finished({ 

 fragmentName: fragmentName, 

 content: data 

 }); 

 } 

 else { 

 finished({error: 'Unable to read file ' + fragPath}); 

 } 

 });

Like it implemented in https://github.com/robtweed/qewd/blob/master/lib/appHandler.js#L63

issue during gtm integration tests with ajax request

Hey @robtweed

Environment:

express
gtm

Please take a look:

Line 292 in cbf87ac

session.socketId = messageObj.socketId;

I found that app crushed on this line with error:

I think this line should be replaced to

if (messageObj.socketId) session.socketId = messageObj.socketId;
if (messageObj.ipAddress) session.ipAddress = messageObj.ipAddress;

WDYT?

empty handler

Do we need to keep it?

qewd/lib/socketClient.js

Lines 49 to 50 in fbf821b

 this.on('error', function(message) { 

 });

application is used before defined/initialized?

@robtweed Hi

Line 115 in 6b9b898

 if (this.errorMessages && this.errorMessages[application] && this.errorMessages[application]['noDocumentStore']) 

should it be messageObj.application?

data is null

Please take a look

qewd/lib/sockets.js

Lines 72 to 77 in cbf87ac

 if (typeof data !== 'object') { 

 console.log('Received message is not an object: ' + data); 

 return; 

 } 

 var type = data.type; 

 if (!type) {

if data is null, the code will fail on line 76

Question about config/params

HI @robtweed

qewd/lib/master.js

Lines 119 to 124 in b74a7c5

 if (params.resilientMode) { 

 config.resilientMode = { 

 documentName: params.resilientMode.queueDocumentName || 'ewdQueue', 

 keepPeriod: params.resilientMode.keepPeriod || 3600 

 } 

 }

Should it be if (params.resilientMode) { or if (config.resilientMode) {?

finished call missed?

@robtweed

Lines 306 to 308 in 6b9b898

 } 

 var responseObj = getFragment.call(this, messageObj, application, finished); 

 return;

should be finished(responseObj) between lines 307/308?

`beforeRouter`/`afterRouter` not needed to be validated for isArray

qewd/lib/master-express.js

Please consider the following lines:

Lines 95 to 102 in cbf87ac

 var options = { 

 nextCallback: route.afterRouter ? true : false 

 }; 

 args = args.concat([qx.router(options)]); 

 // add array with custom middleware to add after qx.router is called (if present) 

 if (route.afterRouter && Array.isArray(route.afterRouter)) args = args.concat(route.afterRouter);

Some notes:

I'm not sure that beforeRouter/afterRouter must be validated to be an array because concat works fine if passed argument can be added to existing array -> [1, 2].concat(3) => [1, 2, 3]
Another note is that, if we keep the current structure and declare beforeRouter/afterRouter as NOT an array, nextCallback can be set incorrectly (it will be true) but afterRouter won't be added

My proposal here is to allow developers to pass afterRouter as function of array of functions. In this case we do not need to validate beforeRouter/afterRouter for Array.isArray

master-koa vs. master-express

koa has 2 routes for ajax (GET and POST)
https://github.com/robtweed/qewd/blob/master/lib/master-koa.js#L94-L101

express has only 2 route for POST
https://github.com/robtweed/qewd/blob/master/lib/master-express.js#L60-L64

@robtweed is it correct?

frag path

jhnns/rewire#116
jhnns/rewire#101

Line 42 in 71a0457

var fragPath = wsRootPath + application + path.sep + fragmentName;

can be refactored to path.join(wsRootPath, application, fragmentName)

update rewire to fix bug #101

serviceModuleLoad error handling never called

The catch block in the following example is never called because loadModule function catch issue inside and returns boolean depends on the results.

Lines 356 to 371 in 71a0457

 try { 

 //this.handlers[service] = require(service).handlers; 

 var ok = loadModule.call(this, service, finished); 

 if (!ok) return; 

 //console.log('service module loaded for ' + service); 

 } 

 catch(err) { 

 error = 'Unable to load handler module: ' + service; 

 if (this.errorMessages && this.errorMessages[application] && this.errorMessages[application]['serviceModuleLoad']) error = this.errorMessages[application]['serviceModuleLoad']; 

 console.log(error + ': ' + err); 

 finished({ 

 error: error, 

 reason: err, 

 service: service 

 }); 

 return;

I think the correct one here is:

pass information about service load to loadModule
move serviceModuleLoad error handling inside loadModule function

ok = loadModule.call(this, service, finished);
if (!ok) return;

NOTE:
The code above doesn't pass information about service or application to loadModule

Please provide your thoughts...

File upload in not working in qewd

Hi,
I want to create a new API for file upload in QEWD Application. I have defined the API in routes and called the API from postman but the request does not have the form-data/multipart file.
Thanks.

unused function

Lines 433 to 440 in 736580d

 function isTokenAPossibleJWT(token) { 

 var pieces = token.split('.'); 

 if (pieces.length !== 3) return false; 

 if (pieces[0].length < 2) return false; 

 if (pieces[1].length < 2) return false; 

 if (pieces[2].length < 2) return false; 

 return true; 

 }

Question about DELETE route for master-koa

Please take a look:

qewd/lib/master-koa.js

Lines 151 to 164 in b74a7c5

 match = 'POST ' + path + '/:type/*'; 

 koaRouter.addRoute(match, args); 

 match = 'POST ' + path + '/:type'; 

 koaRouter.addRoute(match, args); 

 match = 'DELETE ' + path + '/:type/*'; 

 koaRouter.addRoute(match, args); 

 match = 'PUT ' + path + '/:type'; 

 koaRouter.addRoute(match, args); 

 match = 'PUT ' + path + '/:type/*'; 

 koaRouter.addRoute(match, args);

All verbs are used two times for '/:type and '/:type/*' but DELETE is used only once (for '/:type/*'). Is it correct?

errorMessages logic in appHandler can be simplified (reminder)

Qewd-up without websockets?

Is it possible to configure the Orchestrator to forward incoming REST requests to a microservice using REST calls rather than websocket messages?

We are trying to integrate a non-Qewd service into an existing Qewd-up deployment.

Many thanks.
G

return true if success

qewd/lib/worker.js

Lines 78 to 91 in b74a7c5

 this.setCustomErrorResponse = function(params) { 

 var application = params.application; 

 if (!application || application === '') return false; 

 var type = params.errorType; 

 if (!type || type === '') return false; 

 var text = params.text || 'Unspecified Error'; 

 var statusCode = params.statusCode || '400'; 

 if (!this.errorMessages) this.errorMessages = {}; 

 if (!this.errorMessages[application]) this.errorMessages[application] = {}; 

 this.errorMessages[application][type] = { 

 text: text, 

 statusCode: statusCode 

 }; 

 };

I think it would be good if setCustomErrorResponse returns true on success

responseObj.message.error vs. responseObj.error?

Hey @robtweed

Please take a look:

Lines 116 to 124 in b74a7c5

 if (responseObj.message.error) { 

 socket.emit('ewdjs', { 

 type: data.type, 

 message: { 

 error: responseObj.error, 

 disconnect: true 

 } 

 }); 

 return;

First, we check responseObj.message.error but then take error from responseObj.error. Is it correct?

Passing this to beforeMicroServiceHandler

Lines 49 to 52 in b74a7c5

 if (appModule.beforeMicroServiceHandler) { 

 this.beforeHandlers[application] = function(messageObj, session, send, finished) { 

 return appModule.beforeMicroServiceHandler.call(this, messageObj, finished); 

 };

Looks like this passes to beforeMicroServiceHandler is not the same this that passed to appModule.init for example. Could you please take a look?

Handle boolean property correctly

Hey @robtweed

Lines 330 to 340 in 71a0457

 function getProperty(propertyName, token) { 

 var payload; 

 try { 

 payload = jwt.decode(token, null, true); 

 } 

 catch(err) { 

 return false; 

 } 

 if (!payload[propertyName]) return false; 

 return payload[propertyName]; 

 }

What if payload contains false, '' or null? In this case false be returned by getProperty. Maybe it's better to have here if (typeof payload[propertyName] === 'undefined'? Ofc if it makes sense here

Questions about formatting when error handling

Please take a look:
https://gist.github.com/killmenot/690864b4189904054bfcc79c6fea78a8

docker vs. docker-server

Hi Rob,

Could you please explain the different intent for the docker and docker-server folders? (Or perhaps add readme files to those folders.)

unused variable

Line 38 in 71a0457

var targetId = messageObj.params.targetId;

A problem with custom routes

Motivation

I'd like to add a simple module that provided with healthcheck functionality (actually, it's part of integration testing for ripple qewd microservices).

The module should look very simple:
qewd.js

'use strict';

const qewd = require('qewd').master;
const path = require('path');

const config = {
  managementPassword: 'keepThisSecret!',
  serverName: 'My QEWD Server',
  port: 8080,
  database: {
    type: 'redis'
  }
};
const routes = [
  {
    path: '/',
    module: path.join(__dirname, 'healthcheck')
  }
];

qewd.start(config, routes);

healthcheck.js

function healthcheck(args, finished) {
  finished({
    ok: true,
    timestamp: Date.now()
  });
}

module.exports = {
  restModule: true,

  init: function (application) {
    const routes = [
      {
        url: '/healthcheck',
        method: 'GET',
        handler: healthcheck
      }
    ];

    router.initialise(routes, module.exports);
    router.setErrorResponse(404, 'Not Found');

    this.setCustomErrorResponse.call(this, {
      application: application,
      errorType: 'noTypeHandler',
      text: 'Resource Not Found',
      statusCode: '404'
    });
  }
};

Problem

It returns 404

Investigation

I started to look deeper at the problem to see why such a simple case doesn't work. After research master-express, qewd-router and qx.router logic I found the following:
routes.path is used as first parameter to pass in express middleware app.use the following way

app.use(routes.path, beforeRouter, qx.router(options), afterRouter)

That means routes.path should be / to work correctly (see example with 2 routers on root path). However, this leads to another problem. GET /healthcheck returns {"error":"Missing authorization header"}

Looking at express params I found the following:

{
  "type": "ewd-qoper8-express",
  "path": "/healthcheck",
  "method": "GET",
  "headers": {
    "host": "127.0.0.1:8085",
    "connection": "keep-alive",
    "cache-control": "max-age=0",
    "upgrade-insecure-requests": "1",
    "user-agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36",
    "dnt": "1",
    "accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8",
    "accept-encoding": "gzip, deflate, br",
    "accept-language": "en-US,en;q=0.9,ru;q=0.8,de;q=0.7",
    "cookie": "io=gT4u07XmDNfb7crjAAAF"
  },
  "params": {
    "type": "healthcheck"
  },
  "query": {},
  "body": {},
  "ip": "::ffff:172.17.0.1",
  "ips": []
}

And again there are 2 problems here now

module map looks like {"":"/opt/qewd/healthcheck"} that it's bad
application prop is missed for messageObj and corresponding app handler logic is not called

Solution

Update qewd master module map initialization to use application parameter when path is /
I found that application and expressType parameters are not passed to router here. If we update corresponding master-express and master-koa logic, this problem will be gone.

Now messageObj looks correctly

module map is initialized correctly -> moduleMap":{"healthcheck":"/opt/qewd/healthcheck"}
messageObj looks correctly to be loaded by restModules

{
  "type": "ewd-qoper8-express",
  "path": "/healthcheck",
  "method": "GET",
  "headers": {
    "host": "127.0.0.1:8085",
    "connection": "keep-alive",
    "cache-control": "max-age=0",
    "upgrade-insecure-requests": "1",
    "user-agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36",
    "dnt": "1",
    "accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8",
    "accept-encoding": "gzip, deflate, br",
    "accept-language": "en-US,en;q=0.9,ru;q=0.8,de;q=0.7",
    "cookie": "io=gT4u07XmDNfb7crjAAAF"
  },
  "params": {
    "type": "healthcheck"
  },
  "query": {},
  "body": {},
  "ip": "::ffff:172.17.0.1",
  "ips": [],
  "application": "healthcheck",
  "expressType": "healthcheck"
}

However, the module could not be loaded due to the problem with processing name for handler in qewd-router. This should be patched also. If it be patched for this particular case, all will work. The final healthcheck module code is the following: https://gist.github.com/killmenot/44f03452b04066f97baa28ad91f6e279

differences in error handling

If we take a look at no docment store logic, we can see that it uses customError function
https://github.com/robtweed/qewd/blob/master/lib/appHandler.js#L113-L119

However, if we take a look at session is not authenticated logic
https://github.com/robtweed/qewd/blob/master/lib/appHandler.js#L271-L278
we see no customError involved.

Should I add customError logic to session is not authenticated logic?

Question about using resilientMode.documentName in resilientMode.js

Sometimes in resilientMode.js I see different usage of resilientMode.documentName

First way:
https://github.com/robtweed/qewd/blob/master/lib/resilientMode.js#L37

Second way:
https://github.com/robtweed/qewd/blob/master/lib/resilientMode.js#L157

But it looks like they point to the same
https://github.com/robtweed/qewd/blob/master/lib/master.js#L155

I think it can be unified.

Docker Hub name is wrong in README?

Hi Rob,

I'm trying to have a go with QEWD-UP. The read me shows as attached:

I searched docker hub but couldn't find anything with that name?
Also you have a double parenthesis in that bit of README that could probably use a tidy-up :)

unused variable

Line 200 in b74a7c5

payloadsocketId = messageObj.socketId;

Question about socketClient logic

Please take a look at message obj format when socket client connected event is emitted -> https://github.com/robtweed/qewd/blob/master/lib/socketClient.js#L108. connected event emitted ewdjs in the end

However, when ewdjs is emitted, we check message type and then get token by taken messageObj.message.token -> https://github.com/robtweed/qewd/blob/master/lib/socketClient.js#L68

Is it correct?

bug with resilient mode and gtm

Hey @robtweed

Simple test should be able to send message using websockets:

describe('custom message', () => {
    let data;

    beforeEach((done) => {
      request.
        post('/ajax').
        send({
          type: 'ewd-register',
          application: 'test-app'
        }).
        end((err, res) => {
          if (err) return done.fail(err);

          data = {
            type: 'test',
            token: res.body.token,
            params: {
              text: 'Hello world'
            }
          };

          done();
        });
    });

    it('should be able to send message using websockets', (done) => {
      const socket = io.connect('ws://localhost:8080');

      socket.on('connect', () => socket.emit('ewdjs', data));
      socket.on('ewdjs', (responseObj) => {
        socket.disconnect();

        expect(responseObj).toEqual({
          type: 'test',
          finished: true,
          message: {
            text: 'You sent: Hello world via express'
          },
          responseTime: jasmine.stringMatching(/^\d*ms$/)
        });

        done();
      });
    });
  });

QEWD configuration:

express
gtm
resilientMode

Output:

QEWD configuration:

express
redis
resilientMode

Question

Hey @robtweed

Could you please explain me the logic of the following lines?

Lines 303 to 305 in 71a0457

 if (messageObj.service && !this.servicesAllowed[application]) { 

 var ok = loadModule.call(this, application, finished); 

 if (!ok) return;

is it correct to pass application to loadModule? I have a feeling that messageObj.service should be passed there

A problem with processing array responses with workerResponseHandlers