arabold / serverless-sentry-lib

MIT License
32 stars 15 forks source link

AWS Lamda Production not working #3

Closed jnreynoso closed 6 years ago

jnreynoso commented 7 years ago

serverless-sentry-lib works for me in develop but not in production, my configuration of serverless.yml:

service: serverless-api
plugins:
  - serverless-sentry
  - serverless-webpack
  - serverless-offline
provider:
  name: aws
  region: us-east-1
  runtime: nodejs6.10
  stage: production
  environment: '${file(.env.yml):${self:custom.stage}}'
custom:
  stage: '${opt:stage, self:provider.stage}'
  serverless-offline:
    port: 4000
    babelOptions:
      presets:
        - es2015
        - es2017
  sentry:
    dsn: https://(SECRET):(SECRET)@sentry.io/(SECRET)
    filterLocal: true   
  .....

verify that the DSN is correct and that the issues are coming to me (local environment), but the mistakes that I am generating in production do not arrive

'use strict'

const response = require('../../helpers/response')
import User from '../../database/models/user'

var Raven = require('raven')
var RavenLambdaWrapper = require('serverless-sentry-lib')

module.exports.handler = RavenLambdaWrapper.handler(Raven,  (event, context, callback) => {

  context.callbackWaitsForEmptyEventLoop = false

  global.cb = callback

  const userid = event.pathParameters.userid

  User.findOne({ where: { userid: userid }}).then((user) => {
    if(user)
      response('user logged', 200, user)
    else
      response('user logged', 200, {error: true})

  }).catch(error => {
    response('user logged', 200, {error: true, message: error})
  })

})

Generate Error:

 UsesdfsdfdsrfindOne({ where: { userid: userid }}).then((user) => { // UsesdfsdfdsrfindOne is undefined (Exception)
    if(user)
      response('user logged', 200, user)
    else
      response('user logged', 200, {error: true})

  }).catch(error => {
    response('user logged', 200, {error: true, message: error})
  })

Packaje.json

"dependencies": {
    ....
    ....
    "raven": "2.2.1",
    "serverless-sentry-lib": "1.0.0-rc.2"
  },
  "devDependencies": {
    "serverless-sentry": "1.0.0-rc.4",
  }
arabold commented 7 years ago

The problem might be context.callbackWaitsForEmptyEventLoop = false. This will freeze the Lambda even if there are still HTTP callbacks pending or if Sentry/Raven still has some notifications in the queue. We have the exact same problem and unfortunately there's no easy way to solve this. See this feature request for raven-node: https://github.com/getsentry/raven-node/issues/338

For us internally this isn't a big deal, however, as typically the errors will be sent on next invocation of the Lamdba. Yes, sounds a bit weird, but if the error is still queued somewhere there's a good chance that it still sends as expected in the subsequent Lambda call. You can test this by hitting your endpoint multiple times without redeploying in between. The errors should arrive eventually.

jnreynoso commented 7 years ago

Hello @arabold.

I'm using context.callbackWaitsForEmptyEventLoop = false, because if necessary when you work with sequelize. I tried to test what he told me in another function in which I do not use calbackWait ... and the mistakes did not come, although I called him multiple times (endpoint).

it's a bit frustrating ,thanks for your help @arabold.

timmyers commented 6 years ago

I have found that the context.callbackWaitsForEmptyEventLoop = false option does not even seem to be working with this library, due to the fact that internally the library clones context before passing it to the handler parameter.

As a result, setting the property doesn't percolate back up to the true AWS context.

To workaround, I am currently doing something like this:

const wrapperWrapper = (handler) => (event, context, callback) => {
  context.callbackWaitsForEmptyEventLoop = false;
  return handler(event, context, callback);
};

and then wrapping THIS around the library wrapper. I might try to put together a PR to fix this issue.

arabold commented 6 years ago

Oh, that's an interesting problem! I never noticed this issue because we're setting callbackWaitsForEmptyEventLoop outside of the RavenLambdaWrapper.handler. But you're right, if you try to set it inside of the wrapped function, it will not work as expected.

Thanks for the PR. I'm gonna review it and add my thoughts.

demsey2 commented 6 years ago

hi @arabold I have the same problem and I managed to fix that problem with the following code

function shallowClone(obj) {
    var clone = Object.create(Object.getPrototypeOf(obj));
    var props = Object.getOwnPropertyNames(obj);
    props.forEach(function(key) {
        var desc = Object.getOwnPropertyDescriptor(obj, key);
        Object.defineProperty(clone, key, desc);
    });
    return clone;
}
const wrappedCtx   = shallowClone(context);
// you have that done with extend which doesn't clone callbackWaitsForEmptyEventLoop properly (it's a setter and getter)
// const wrappedCtx   = _.extend({}, context);

are you going to provide any fix for this issue? can you provide an example of your function where allbackWaitsForEmptyEventLoop is outside of the RavenLambdaWrapper.handler?

arabold commented 6 years ago

I've just published v1.0.1. It would be great if you can give it a shot!

sverraest commented 6 years ago

@timmyers Could you post an example on how you were able to wrap your solution around this library's wrapper?