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

transaction rollback not executed #197

Closed
mohanagy opened this issue Apr 4, 2019 · 2 comments
Closed

transaction rollback not executed #197

mohanagy opened this issue Apr 4, 2019 · 2 comments
Labels

Comments

@mohanagy
Copy link

mohanagy commented Apr 4, 2019

I'm using Knex transactions in hooks and I followed the documentation which says

 module.exports = {
   before: {
    all: [ transaction.start() ],
    find: [],
    get: [],
    create: [],
    update: [],
    patch: [],
    remove: []
  },
  after: {
    all: [ transaction.end() ],
    find: [],
    get: [],
    create: [],
    update: [],
    patch: [],
    remove: []
  },
  error: {
    all: [ transaction.rollback() ],
    find: [],
    get: [],
    create: [],
    update: [],
    patch: [],
    remove: []
  }
};

I'm using multi services and I call services through another service hook
the problem is if service failds ,for example there was duplicated row
it shouden't continue and should rollaback
but this not happen
this is my code :
registration.service.js

// Initializes the `users` service on path `/registeration`
const createService = require('feathers-knex');
const createModel = require('../../../models');
const hooks = require('./registration.hooks');

module.exports = function (app) {
  const Model = createModel(app);
  const cognitoPool = app.get('cognitoPool');

  const options = {
    name: 'Accounts',
    Model,
    paginate:false,
    cognitoPool
  };

  // Initialize our service with any options it requires
  app.use('/registration', createService(options));

  // Get our initialized service so that we can register hooks and filters
  const service = app.service('registration');

  service.hooks(hooks(options));
};

registration.hooks.js

const validate = require('feathers-hooks-validate-joi');
const { registration } = require('../../../validations');
const { hooks } = require('feathers-knex');
const { transaction } = hooks;
const { registration: { beforeRegistration, afterRegistration } } = require('../../../hooks/auth');

module.exports = (options) => ({
  before: {
    all: [transaction.start()],
    find: [],
    get: [],
    create: [
      validate.form(registration.registrationSchema, registration.joiOptions),
      beforeRegistration()
    ],
    update: [],
    patch: [],
    remove: []
  },

  after: {
    all: [transaction.end()],
    find: [],
    get: [],
    create: [afterRegistration(options),]
    ,
    update: [],
    patch: [],
    remove: []
  },

  error: {
    all: [transaction.rollback()],
    find: [],
    get: [],
    create: [],
    update: [],
    patch: [],
    remove: []
  }
});

beforeRegisration.js

const errors = require('@feathersjs/errors');
// Use this hook to manipulate incoming or outgoing data.
// For more information on hooks see: http://docs.feathersjs.com/api/hooks.html

// eslint-disable-next-line no-unused-vars
module.exports = function (options = {}) {
    return async context => {
        try {
            const { data: { company_name, account_type, email, first_name, last_name, phone_number, password } } = context
            const userData = {
                first_name,
                last_name,
                email,
                phone_number,
                password
            }
            const newContext = {
                ...context,
                data: {
                    company_name,
                    account_type,
                    created_by: 1,
                    updated_by: 1
                },
                userData
            }
            return newContext
        } catch (e) {
            throw errors.FeathersError({
                error: e,
                success: false
            })
        }

    };
};


afterRegistration.js

const errors = require('@feathersjs/errors');
// Use this hook to manipulate incoming or outgoing data.
// For more information on hooks see: http://docs.feathersjs.com/api/hooks.html

// eslint-disable-next-line no-unused-vars
module.exports = function (options = {}) {
    return async context => {
        try {
            const { result, userData, params } = context
            if (!result)
                return context
            const { id: account_id } = result
            const data = {
                ...userData,
                account_id,
                created_by: 1,
                updated_by: 1,
            }
            delete data.password
            const newContext = { ...context, data }
            await context.app.service('/users').create(data, { ...params,userData, noToken: true })
            return newContext
        } catch (e) {
            throw errors.FeathersError({
                error: e,
                success: false
            })

        }

    };
};

and this is the users.hooks.js

const validate = require('feathers-hooks-validate-joi');
const { users } = require('../../validations');
const { hooks } = require('feathers-knex');
const { transaction } = hooks;
const { verifyToken } = require('../../hooks/auth')
const { beforeCreateUsers, afterCreateUsers } = require('../../hooks/users');

module.exports = (options) => ({
  before: {
    all: [verifyToken(options), transaction.start()],
    find: [],
    get: [],
    create: [validate.form(users.createUsersSchema, users.joiOptions),beforeCreateUsers()],
    update: [],
    patch: [],
    remove: []
  },

  after: {
    all: [transaction.end()],
    find: [],
    get: [],
    create: [afterCreateUsers(),]
    ,
    update: [],
    patch: [],
    remove: []
  },

  error: {
    all: [transaction.rollback()],
    find: [],
    get: [],
    create: [],
    update: [],
    patch: [],
    remove: []
  }
});

Expected behavior

rollback when error happened

Actual behavior

not rolling back

System configuration

Tell us about the applicable parts of your setup.

Module versions (especially the part that's not working):

dependencies": {
    "@feathersjs/authentication": "^2.1.16",
    "@feathersjs/authentication-jwt": "^2.0.10",
    "@feathersjs/configuration": "^2.0.6",
    "@feathersjs/errors": "^3.3.6",
    "@feathersjs/express": "^1.3.1",
    "@feathersjs/feathers": "^3.3.1",
    "@feathersjs/socketio": "^3.2.9",
    "amazon-cognito-identity-js": "^3.0.9",
    "compression": "^1.7.3",
    "cors": "^2.8.5",
    "feathers-hooks-validate-joi": "^2.0.0",
    "feathers-knex": "^5.0.5",
    "feathers-logger": "^0.3.2",
    "helmet": "^3.15.1",
    "joi": "^14.3.1",
    "joi-date-extensions": "^1.2.0",
    "jsonwebtoken": "^8.5.1",
    "jwk-to-pem": "^2.0.1",
    "knex": "^0.16.3",
    "knex-waitfordb": "^1.1.0",
    "moment": "^2.24.0",
    "mysql": "^2.16.0",
    "mysql2": "^1.6.5",
    "node-fetch": "^2.3.0",
    "parse-error": "^0.2.0",
    "serve-favicon": "^2.5.0",
    "winston": "^3.2.1"
  },
  "devDependencies": {
    "eslint": "^5.13.0",
    "mocha": "^5.2.0",
    "nodemon": "^1.18.10",
    "request": "^2.88.0",
    "request-promise": "^4.2.2"
  }
}

NodeJS version:
v10.15.3

Operating System:
ubuntu

kind Regards

@daffl daffl added the bug label Apr 5, 2019
@vonagam
Copy link
Contributor

vonagam commented Jun 10, 2019

I don't know if this is a cause of your issue. But from your code i see one problem.

In registration.hooks.js you have:

after: {
  all: [transaction.end()],
  ...
  create: [afterRegistration(options)],
  ...
}

And afterRegistration hook calls users service with ...params, i assume to share a transaction. But the problem is that all hooks are executed first and then specific method ones. And in this case a transaction is committed and removed from params with transaction.end hook, so users service will not share the transaction, and if users service fails then you end up with a created registration and no user.

@daffl
Copy link
Member

daffl commented Jun 10, 2019

Should be fixed via #206 in v5.1.0. Thanks @vonagam

@daffl daffl closed this as completed Jun 10, 2019
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

3 participants