-
Notifications
You must be signed in to change notification settings - Fork 211
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
Comments
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 Here's the logic for this: The simplest order that always does the right thing might be: 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. |
Thank you. Really appreciate your feedback.
FYI, leaving this comment by replying to the email from Github. Hopefully
this works as commenting online still does not.
…On Thu, Jan 5, 2023 at 4:08 PM Walt Jones ***@***.***> wrote:
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.
—
Reply to this email directly, view it on GitHub
<#1088 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKDDFTQG3X6GOJN7MWVFB3WQ5H7DANCNFSM6AAAAAATSONRCU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Good to hear, and thank you for the follow up about the commenting issue. |
@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:
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:
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:But I suppose I might want to have the flexibility to pass a more specific payload like so:
Are you saying in this second example above that the
req
object would need to be the first argument?The text was updated successfully, but these errors were encountered: