Skip to content

Conversation

@sangeethailango
Copy link
Member

@sangeethailango sangeethailango commented Oct 23, 2025

Description

Added a try-except block on crawl_work_item_link_title, which handles DoesNotExist error for IssueLink

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

References

Sentry

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling for background work item link title updates: when a referenced link is missing, the system now logs the event and exits that update path gracefully instead of failing. This prevents unnecessary errors, reduces noise in logs, and improves stability and reliability of background processing that updates link titles.

@sangeethailango sangeethailango added the 🐛bug Something isn't working label Oct 23, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

Added try/except around IssueLink.objects.get(...) in the work item link crawl function; if the IssueLink is missing the function logs a warning and returns early. When found, metadata is assigned and the instance is saved as before.

Changes

Cohort / File(s) Summary
Error handling for missing IssueLink
apps/api/plane/bgtasks/work_item_link_task.py
Wrapped IssueLink.objects.get(...) in a try/except for IssueLink.DoesNotExist; on exception the code logs a warning and returns early. When found, metadata is assigned and the object is saved as before.

Sequence Diagram(s)

sequenceDiagram
  participant Scheduler
  participant Task as crawl_work_item_link_title
  participant DB as IssueLink.objects.get
  Scheduler->>Task: enqueue / trigger
  Task->>DB: get(issue_link_id)
  alt IssueLink exists
    DB-->>Task: IssueLink instance
    Task->>Task: assign metadata\nsave()
    Task-->>Scheduler: complete
  else DoesNotExist
    DB-->>Task: DoesNotExist
    Task->>Task: log warning\nreturn early
    Task-->>Scheduler: complete (early)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Review apps/api/plane/bgtasks/work_item_link_task.py to confirm logging content and that early return preserves expected task lifecycle.

🐰
I hopped to check a link one night,
A missing bug gave quite a fright.
I logged a warning, tipped my hat,
Returned with grace — no crash, just chat.
Small fix, big calm, we nibble on. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description includes the required "Description" section explaining the try-except block addition, the "Type of Change" section with "Bug fix" checked, and the "References" section with a Sentry link provided. However, the "Test Scenarios" section is completely empty and not filled out. According to the repository template, this is a required section where the author should describe the tests performed to verify the changes. For a bug fix PR, this section is particularly important to demonstrate the fix was properly validated before merging. The author must add detailed test scenarios in the "Test Scenarios" section to explain how the IssueLink.DoesNotExist exception handling was tested. This should include specific steps taken to verify the fix resolves the issue and doesn't introduce regressions.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "[WEB-5228] chore: IssueLink.DoesNotExist on crawl_work_item_link_title" clearly and specifically describes the main change in the pull request. According to the raw summary, the primary modification is adding a try-except block to handle IssueLink.DoesNotExist errors in the crawl_work_item_link_title function, which matches what the title conveys. The title is concise, avoids vague terms, and provides enough context for a reviewer to understand the focus of the change without needing to inspect the diff.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore-sentry-issuelink-not-exist

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18d29a2 and 6721ecc.

📒 Files selected for processing (1)
  • apps/api/plane/bgtasks/work_item_link_task.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/api/plane/bgtasks/work_item_link_task.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build and lint web apps
  • GitHub Check: Analyze (javascript)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sangeethailango sangeethailango changed the title chore: IssueLink.DoesNotExist on crawl_work_item_link_title [WEB-5228] chore: IssueLink.DoesNotExist on crawl_work_item_link_title Oct 23, 2025
@makeplane
Copy link

makeplane bot commented Oct 23, 2025

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

@sangeethailango sangeethailango marked this pull request as ready for review October 23, 2025 12:28
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/api/plane/bgtasks/work_item_link_task.py (2)

175-179: Include the id in the log message for better debugging.

The log message should include the id parameter to help trace which IssueLink was not found, especially when investigating issues.

Apply this diff to improve the log message:

-        logger.info("IssueLink not found")
+        logger.info(f"IssueLink not found for id: {id}")

173-182: Consider validating meta_data before saving to avoid persisting error states.

The crawl_work_item_link_title_and_favicon function returns a dict with an "error" key when crawling fails (lines 78-83). Currently, this error response would be saved to the database. Consider checking if the crawl was successful before persisting the metadata.

Apply this diff to validate the metadata before saving:

     meta_data = crawl_work_item_link_title_and_favicon(url)
+    
+    # Skip saving if crawling failed
+    if "error" in meta_data:
+        logger.warning(f"Failed to crawl link metadata for IssueLink {id}: {meta_data.get('error')}")
+        return
 
     try:
         issue_link = IssueLink.objects.get(id=id)
     except IssueLink.DoesNotExist:
-        logger.info("IssueLink not found")
+        logger.info(f"IssueLink not found for id: {id}")
         return
 
     issue_link.metadata = meta_data
     issue_link.save()
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e710f5b and fb7865b.

📒 Files selected for processing (1)
  • apps/api/plane/bgtasks/work_item_link_task.py (1 hunks)

@pushya22 pushya22 merged commit 69fe581 into preview Oct 28, 2025
6 of 7 checks passed
@pushya22 pushya22 deleted the chore-sentry-issuelink-not-exist branch October 28, 2025 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛bug Something isn't working ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants