Skip to content

refactor: create integration request in new transaction; code smell; #55

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.

Sign up for GitHub

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

Open
karm1000 wants to merge 4 commits into
base: version-15
Choose a base branch
from
Open

refactor: create integration request in new transaction; code smell; #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3be17e
refactor: create integration request in new transaction; code smell;
karm1000 Oct 3, 2025
f6ae156
fix: add nosemgrep comment to database commit line
karm1000 Oct 3, 2025
4854a77
fix: correct import
karm1000 Oct 6, 2025
1995b67
refactor: improve transaction handling in execute_in_new_transaction
karm1000 Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions transaction_parser/transaction_parser/ai_integration/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
)
from transaction_parser.transaction_parser.utils import is_enabled, to_dict
from transaction_parser.transaction_parser.utils.integration_request import (
enqueue_integration_request,
create_integration_request,
)


Expand Down Expand Up @@ -79,7 +79,7 @@ def send_message(self, messages: tuple, file_doc_name: str | None = None) -> dic
raise e

finally:
enqueue_integration_request(**log)
create_integration_request(**log)
Comment on lines 81 to +82
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Wrap in try-except to prevent exception masking.

Creating the integration request in a separate transaction is good for observability. However, if create_integration_request raises an exception in the finally block, it will replace any exception raised earlier (line 79), making debugging difficult.

Apply this diff to prevent exception masking:

         finally:
-            create_integration_request(**log)
+            try:
+                create_integration_request(**log)
+            except Exception:
+                frappe.log_error("Failed to create integration request")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
finally:
enqueue_integration_request(**log)
create_integration_request(**log)
finally:
try:
create_integration_request(**log)
except Exception:
frappe.log_error("Failed to create integration request")
🤖 Prompt for AI Agents 
In transaction_parser/transaction_parser/ai_integration/parser.py around lines
81-82, the finally block calls create_integration_request which can raise and
mask an earlier exception; wrap the create_integration_request call in a
try/except that catches and logs the secondary exception (including traceback
and context) but does not overwrite the original error — either suppress the
secondary exception after logging or attach it via chaining, and ensure the
original exception (if present) is re-raised so the root cause is preserved.


def _create_log_entry(self, file_doc_name: str | None) -> frappe._dict:
"""Create a log entry for the API call."""
Expand Down
18 changes: 18 additions & 0 deletions transaction_parser/transaction_parser/utils/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
from functools import wraps

import frappe
from frappe import _
Expand Down Expand Up @@ -33,3 +34,20 @@ def to_dict(value, throw=True):
frappe.throw(_("Invalid JSON"))

return frappe._dict()


def execute_in_new_transaction(fn):
@wraps(fn)
def wrapper(*args, **kwargs):
_db = frappe.local.db
try:
frappe.connect(set_admin_as_user=False)
result = fn(*args, **kwargs)
frappe.db.commit() # nosemgrep
return result

finally:
frappe.db.close()
frappe.local.db = _db

return wrapper
10 changes: 5 additions & 5 deletions transaction_parser/transaction_parser/utils/integration_request.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
import frappe
from frappe import _

from transaction_parser.transaction_parser.utils.__init__ import pretty_json
from transaction_parser.transaction_parser.utils import (
execute_in_new_transaction,
pretty_json,
)

SERVICE_NAME = "Transaction Parser API"


def enqueue_integration_request(**kwargs):
frappe.enqueue(create_integration_request, **kwargs)


@execute_in_new_transaction
def create_integration_request(
url=None,
request_id=None,
Expand Down