-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat(minor)!: Add document reference to Error Log and doc.log_error #16653
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #16653 +/- ##
===========================================
- Coverage 57.37% 53.42% -3.95%
===========================================
Files 762 749 -13
Lines 68145 67363 -782
Branches 5890 5711 -179
===========================================
- Hits 39098 35991 -3107
- Misses 25464 27226 +1762
- Partials 3583 4146 +563
Flags with carried forward coverage won't be shown. Click here to find out more. |
@@ -1052,4 +1052,4 @@ def attach_files_to_document(doc, event): | |||
folder="Home/Attachments", | |||
).insert() | |||
except Exception: | |||
frappe.log_error(title=_("Error Attaching File")) | |||
file_doc.log_error("Error Attaching File") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling this exception leads to a NameError
since the variable file_doc
won't be defined if the insert isn't successful.
Will fix this via #16289
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gavindsouza !
@@ -2068,25 +2068,36 @@ def logger( | |||
) | |||
|
|||
|
|||
def log_error(message=None, title=_("Error")): | |||
def log_error(title=None, message=None, reference_doctype=None, reference_name=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this order switched? There are 100s of log_error calls who will check and fix them all? 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @ankush. It's used everywhere. Will make a lot of error handling fail. Doing this will also make the API inconsistent, since msgprint
still has the msg
first.
reference_doctype
,reference_name
) to be able to track and filter source of errordoc.log_error(title, message)