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

Continuation of issue 1084 #1088

Open
npearson72 opened this issue Jan 5, 2023 · 3 comments
Open

Continuation of issue 1084 #1088

npearson72 opened this issue Jan 5, 2023 · 3 comments

Comments

@npearson72
Copy link

@waltjones Not sure what's going on with Github (or perhaps it's an issue with this repo), but I can't seem to leave a reply on issue: #1084

It's throwing an error when I do:

Screen Shot 2023-01-05 at 3 29 35 PM

I'll post this new issue here to continue that discussion:

So given that I am not using the middleware error handler workflow, but still want to have the request object passed into error notifications to Rollbar, I wrapped my rollbar instance in a middlware function like so:

const levels = ['critical', 'debug', 'error', 'info', 'warning']

const appendRollbarInstance = (req, _res, next) => {
  for (const level of levels) {
    rollbarInstance[level] = (...args) => {
      return Rollbar.prototype[level].call(rollbarInstance, ...args, req);
    };
  }

  return next();
};

Do you see any issues with this?

The only issue I'm seeing with this approach is that you mentioned the req object can be passed in as the first argument sometimes. Currently I'm just passing in errors like so:

try {
...
} catch (err) {
  rollbarIntance.error(err)
}

But I suppose I might want to have the flexibility to pass a more specific payload like so:

try {
...
} catch (err) {
  rollbarIntance.error({ payload: { errorCode: 'some-code' } }, err)
}

Are you saying in this second example above that the req object would need to be the first argument?

@waltjones
Copy link
Contributor

I misspoke slightly. It should usually work in any order of err, req, and custom object. Leading with the request arg can help prevent false positives on a different argument, since to detect the request, it matches on keys that can be pretty generic.

If the custom data object has for example a body key, and a request hasn't been detected yet, it will be mistakenly detected as the request. So generally, yes I would prefer to put the request before other object args and prevent this edge case.

Here's the logic for this:
https://github.com/rollbar/rollbar.js/blob/master/src/utility.js#L461-L479

The simplest order that always does the right thing might be: Rollbar.prototype[level].call(rollbarInstance, req, ...args)

I'm not sure about the commenting error. It seems like a github issue, but I'll keep an eye out. Thank you for the heads up.

@npearson72
Copy link
Author

npearson72 commented Jan 5, 2023 via email

@waltjones
Copy link
Contributor

Good to hear, and thank you for the follow up about the commenting issue.

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

2 participants