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

Poor error reporting with parse-server, express and Rollbar #2841

Closed
alexblack opened this issue Oct 6, 2016 · 10 comments
Closed

Poor error reporting with parse-server, express and Rollbar #2841

alexblack opened this issue Oct 6, 2016 · 10 comments

Comments

@alexblack
Copy link

From what I can tell, parse-server doesn't throw standard Javascript errors, see: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Error

Instead it throws maps like this:

{
  "message": "code=209, message=invalid session token"
}

Standard error reporting solutions like AirBrake and Rollbar do not handle these well, making it impossible to find out what actually went wrong.

Instead code like this would work well:

var err = new Error('Invalid session token (Code=209)');
err.code = 209; 
@alexblack
Copy link
Author

alexblack commented Oct 11, 2016

@alexblack alexblack changed the title Exceptions - throw new Error('Whoops!') Poor error reporting with parse-server, express and Rollbar Oct 11, 2016
@alexblack
Copy link
Author

alexblack commented Oct 11, 2016

This seems to fix things up a bit:

// Fix up Parse.Error handling for rollbar :(
app.use(function(err, req, res, next) {
  var data = null;

  if (err instanceof Parse.Error) {
    var parseErr = err;
    // replace error
    err = new Error(parseErr.message);
    data = { code: parseErr.code };
    // Group errors by parse error code, see: https://rollbar.com/docs/grouping-algorithm/
    if (parseErr.code)
      data['fingerprint'] = parseErr.code.toString()
  }

  return rollbar.handleErrorWithPayloadData(err, data, req, function(err) {
    return next(err, req, res);
  });
});

@flovilmart
Copy link
Contributor

@alexblack what do you suggest?

@alexblack
Copy link
Author

alexblack commented Oct 22, 2016

If Parse.Error extended from Error as is normal for javascript errors, then that would be a big improvement. But, presumably there is some reason it doesn't.

For us, the code above fixes things up enough.

@flovilmart
Copy link
Contributor

I believe the main issue is that we're using the Parse client SDK errors inside the server. If it makes sense to use a new Error type like ParseServer.Error that extends error I would accept that too if that makes sense. Would you like to work on such PR?

@alexblack
Copy link
Author

How do we figure out if that makes sense? This doesn't sound like a good first PR for someone like myself, as it probably affects a lot of code.

@flovilmart
Copy link
Contributor

That's a replacement at large of all Parse.Error to ParseServer.Error, tedious but not complicated. I don't really have time to work on that for now (I'm trying not to improve the bandwidth for large push sending), and given that you have your workaround, that seems to be OK.

@alexblack
Copy link
Author

Yes the workaround works fine. It lacks stack traces that you normally get with JavaScript, but it's not the end of the world

@alexblack
Copy link
Author

Yes the workaround works fine. It lacks stack traces that you normally get
with JavaScript, but it's not the end of the world.

On Oct 22, 2016 10:27 AM, "Florent Vilmart" [email protected]
wrote:

That's a replacement at large of all Parse.Error to ParseServer.Error,
tedious but not complicated. I don't really have time to work on that for
now (I'm trying not to improve the bandwidth for large push sending), and
given that you have your workaround, that seems to be OK.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2841 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJFwtlST2Fy6sGl-sHWAuIegwubkQ4Wks5q2kdtgaJpZM4KQf6H
.

@acinader
Copy link
Contributor

dupe of #661

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

No branches or pull requests

3 participants