-
Notifications
You must be signed in to change notification settings - Fork 154
Feature request: include error own properties in logged errors #2170
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
Comments
Hi @everett1992, thanks for opening the issue. The format that you described as a bug aligns with the one we specify in the docs. We purposefully standardize the format of an error whenever encountering one during the parsing and normalize the structure to a common denominator. With this in mind, the behavior you're seeing is not a bug. All Using a custom log formatter you can customize the format of the error by overriding the Here you have an example of how to use a custom log formatter. At the moment we only expose the |
Where is it specified in the docs? I read that section but I don't see any claim that only location, stack, name, and message are logged. The examples may only have those fields, but the errors wouldn't have any extra fields. I think 'field', or 'code', 'path', 'errno', 'syscall' should be logged along with the location, message, name and stack. |
Hello everybody! I was reading this issue and thought it was important to give my impressions here as we added this same feature in Powertools Python recently. Firstly, I must say that I agree that Powertools Typescript should add the same capability as Python, that is, in case of exceptions, create a specific field with the entire stack trace to ease discoveries when searching in respective logs. On the other hand, I think that we should not add fields that are not present in all exceptions, as we may incur an increase in the storage of client logs with information that may not be useful for that specific client and that is optional. I was reading the NodeJS errors documentation (https://nodejs.org/api/errors.html#class-systemerror) and saw that fields like As additional context, I personally really like Sentry's exception-catching implementation, and in Python we try to follow the same stack trace format that they implement - of course removing fields that doesn't matter for us. Exampleapp.pyimport sentry_sdk
sentry_sdk.init(
dsn="....",
)
try:
open("leo.txt")
except Exception as e:
sentry_sdk.capture_exception(e) log.json - no personal information was disclosed{
"event_id": "7b36ed6f32874c249e0574197cb23e43",
"exception": {
"values": [
{
"type": "FileNotFoundError",
"value": "[Errno 2] No such file or directory: 'leo.txt'",
"stacktrace": {
"frames": [
{
"function": "<module>",
"module": "__main__",
"filename": "leo.py",
"abs_path": "/home/leandro/DEVEL-PYTHON/tmp/sentry-error/leo.py",
"lineno": 19,
"pre_context": [
")",
"",
"",
"",
"try:"
],
"context_line": " open(\"leo.txt\")",
"post_context": [
"except Exception as e:",
" # Alternatively the argument can be omitted",
" sentry_sdk.capture_exception(e)"
],
"in_app": true,
"vars": {
"__annotations__": {},
"__builtins__": "<module 'builtins' (built-in)>",
"__cached__": "None",
"__doc__": "None",
"__file__": "'/home/leandro/DEVEL-PYTHON/tmp/sentry-error/leo.py'",
"__loader__": "<_frozen_importlib_external.SourceFileLoader object at 0x7f5112e8dc10>",
"__name__": "'__main__'",
"__package__": "None",
"__spec__": "None",
"sentry_sdk": "<module 'sentry_sdk' from '/home/leandro/DEVEL-PYTHON/tmp/sentry-error/.venv/lib/python3.9/site-packages/sentry_sdk/__init__.py'>"
}
}
]
},
"mechanism": {
"type": "generic",
"handled": true,
"meta": {
"errno": {
"number": 2
}
}
}
}
]
}
} Thank you |
That seems like a good compromise, but this still feels like an important feature or bug. Here's examples from other loggers, they each include the error's properties.
|
@heitorlessa would like to hear your opinion on this, thanks. |
This comment has been minimized.
This comment has been minimized.
This behavior is somewhat surprising. Most problematic however, is how it excludes the These have been standard since Node 16 (with feature flags) and the exclusion really hampers debugging. A concrete example: Let's say you're using powertools alongside the secretsmanager middy middleware. If permissions are not set on the KMS key or Secret resource to grant your lambda read/decrypt permissions, middy will throw an error with the following message: Actual log observed (running on a local node process, so some lambda context values are not set): {
"cold_start": true,
"function_memory_size": null,
"level": "ERROR",
"message": "example",
"service": "service_undefined",
"timestamp": "2024-05-10T09:28:17.412Z",
"error": {
"name": "Error",
"location": "/Users/bilalq/example/node_modules/@middy/util/index.cjs:116",
"message": "Failed to resolve internal values",
"stack": "Error: Failed to resolve internal values\n at getInternal (/Users/bilalq/example/node_modules/@middy/util/index.cjs:116:15)\n at processTicksAndRejections (node:internal/process/task_queues:95:5)\n at async secretsManagerMiddlewareBefore (/Users/bilalq/example/node_modules/@middy/secrets-manager/index.cjs:56:26)\n at async runMiddlewares (/Users/bilalq/example/node_modules/@middy/core/index.cjs:126:21)\n at async runRequest (/Users/bilalq/example/node_modules/@middy/core/index.cjs:86:9)"
}
} The Can we maybe at least get |
Hi @bilalq - which version of Logger are you using? We support the cause property unless there's a bug or it's being specified differently than what you linked: |
Ah, that was my mistake. Thanks for pointing it out. It seems that the service I was looking at happened to be on the 1.8.0 version of powertools. The latest version does indeed behave as expected with |
Thanks for confirming - and also for reviving this thread. I just wanted to share that I have not forgotten about this and I am going to try to have a final answer in the next few weeks. Since my initial comment I actually got bit by this myself trying to log a custom error I made and not seeing its properties. I'm now leaning towards including them but I still want to get the alignment of other Powertools runtimes before moving forward. |
This issue is now closed. Please be mindful that future comments are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so. |
This is now released under v2.10.0 version! |
Expected Behaviour
I expect powertools to log all enumerable properties on an error. It's common to include a code, and extra information about an error. For example, node's fs errors include
code
,syscall
andpath
. I'd like powertools to logIf you log a regular object or classs it's properties will be logged, but if the class extends Error only stack, message, location, name, and cause will be logged.
Current Behaviour
logger does not log any of the errors enumerable properties
Code snippet
Logging an error thrown by node's built in fs module, see that
bare
, which is how most people would log an error, is missing errno, code, syscall, and path metadata.spread, and json include the metadata, but not the stack trace, message, or location. spread and json show that these are enumerable properties on the object.
Here's a simple custom error and it has the same problem.
This is logging a normal class that doesn't extend Error and you can see that
bare
includes the properties I would expect.Steps to Reproduce
The script included above reproduces with powertools 1.18.1 or 2.0.2 on node 20.
Possible Solution
Include enumerable properties by spreading the err object.
Powertools for AWS Lambda (TypeScript) version
2.0.2, 1.18.1
AWS Lambda function runtime
20.x
Packaging format used
npm
Execution logs
The text was updated successfully, but these errors were encountered: