Skip to content

Error handling - #1

Open
ShashankFC wants to merge 2 commits into
mainfrom
shashank/error-handling
Open

ShashankFC wants to merge 2 commits into
mainfrom
shashank/error-handling

Conversation

@ShashankFC

@ShashankFC ShashankFC commented Nov 24, 2025

Copy link
Copy Markdown
Collaborator

EntelligenceAI PR Summary

Enhanced error handling in test statistics upload script to prevent crashes and improve robustness during test data processing.

  • Wrapped XML parsing in try-except blocks to gracefully skip unparseable reports with warning messages
  • Added defensive validation checks for element attributes before updating
  • Implemented safe dictionary access using .get() with default values for 'time' field
  • Added exception handling in test case aggregation loop to continue processing on individual failures
  • Wrapped main get_tests() call in try-except to return empty list instead of crashing the entire script

@entelligence-ai-pr-reviews

Copy link
Copy Markdown

Entelligence AI Vulnerability Scanner

Status: No security vulnerabilities found

Your code passed our comprehensive security analysis.

Analyzed 1 files in total

@entelligence-ai-pr-reviews

Copy link
Copy Markdown

Review Summary

🏷️ Draft Comments (6)

Skipped posting 6 draft comments that were valid but scored below your review threshold (>=13/15). Feel free to update them here.

tools/stats/upload_test_stats.py (6)

54-56: parse_xml_report uses a bare except: which will catch and silence all exceptions, including KeyboardInterrupt and SystemExit, potentially hiding critical errors and making debugging difficult.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 5/5
  • Urgency Impact: 2/5
  • Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In tools/stats/upload_test_stats.py, lines 54-56, replace the bare 'except:' in parse_xml_report with 'except Exception:' to avoid catching system-exiting exceptions and to ensure only real parsing errors are handled gracefully.

226-226: In summarize_test_cases, ret[key]["time"] += test_case.get("time", 0) can raise a TypeError if test_case["time"] is not numeric, causing aggregation to fail for all subsequent test cases.

📊 Impact Scores:

  • Production Impact: 4/5
  • Fix Specificity: 5/5
  • Urgency Impact: 3/5
  • Total Score: 12/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In tools/stats/upload_test_stats.py, line 226, wrap the addition of test_case['time'] in a try/except block and cast to float to prevent TypeError or ValueError if the value is not numeric, ensuring aggregation does not break.

210-228: try-except inside the loop in summarize_test_cases (lines 210-228) incurs significant performance overhead for large datasets; exception handling should be outside the loop for efficiency.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 3/5
  • Urgency Impact: 2/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In tools/stats/upload_test_stats.py, lines 210-228, the function summarize_test_cases uses a try-except block inside a loop, which causes significant performance overhead when processing large datasets. Refactor the code to remove the try-except from inside the loop. Instead, validate and handle only the specific cases (such as non-numeric 'time' values) that may cause errors, and ensure that the aggregation logic is robust and efficient. Do not use a blanket try-except inside the loop.

155-169: Appending results from multiprocessing with test_cases.append(mp.apply_async(...)) and then calling [tc.get() for tc in test_cases] is inefficient for large datasets; use imap_unordered or map for better parallelism and memory usage.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 4/5
  • Urgency Impact: 2/5
  • Total Score: 8/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In tools/stats/upload_test_stats.py, lines 155-169, the current multiprocessing pattern appends apply_async results to a list and then calls .get() on each, which is inefficient for large numbers of XML files. Refactor this to use Pool.map or imap_unordered to process all XML files in parallel and collect results efficiently, reducing memory usage and improving parallelism.

80-137: The function process_xml_element (lines 80-137) is overly complex (C901: 11 > 10), making it hard to maintain and optimize; this impacts long-term maintainability and scalability.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 3/5
  • Urgency Impact: 2/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In tools/stats/upload_test_stats.py, lines 80-137, the function process_xml_element is overly complex (C901: 11 > 10), making it difficult to maintain and optimize. Refactor this function to reduce its cyclomatic complexity, possibly by extracting attribute conversion, text/tail handling, and child element processing into separate helper functions. Ensure the refactored code remains functionally equivalent and easier to maintain.

53-53: ET.parse(report) parses untrusted XML without protection, allowing XML External Entity (XXE) attacks that can leak files or cause DoS if attacker controls XML input.

📊 Impact Scores:

  • Production Impact: 4/5
  • Fix Specificity: 3/5
  • Urgency Impact: 4/5
  • Total Score: 11/15

🤖 AI Agent Prompt (Copy & Paste Ready):

Replace the use of `xml.etree.ElementTree` with `defusedxml.ElementTree` in tools/stats/upload_test_stats.py at line 53. This prevents XML External Entity (XXE) attacks when parsing untrusted XML files. Update the import and ensure all XML parsing uses the defusedxml version.

@entelligence-ai-pr-reviews

Copy link
Copy Markdown

Walkthrough

This PR significantly improves the robustness and error handling of the test statistics upload script. The changes focus on preventing script crashes by adding defensive programming practices throughout the codebase. Key improvements include wrapping XML parsing operations in try-except blocks to handle malformed reports gracefully, adding validation checks before accessing element attributes, using safe dictionary access methods with default values, and implementing error handling around the test case aggregation logic. These enhancements ensure that individual failures during processing don't cause the entire upload operation to fail, allowing the script to continue processing remaining test data while logging appropriate warnings.

Changes

File(s) Summary
tools/stats/upload_test_stats.py Added comprehensive error handling including try-except blocks around XML parsing to skip unparseable reports with warnings, defensive validation checks for element attributes, safe .get() method usage with default values for the 'time' field, exception handling in test case aggregation loop to continue on individual failures, and error handling around main get_tests() call to return empty list instead of crashing.

Sequence Diagram

This diagram shows the interactions between components:

sequenceDiagram
    participant Main as Main Script
    participant GT as get_tests()
    participant XML as XML Parser
    participant PE as process_xml_element()
    participant AGG as Aggregation Logic

    Main->>GT: get_tests(workflow_id, attempt)
    activate GT
    
    loop For each report file
        GT->>XML: ET.parse(report)
        activate XML
        
        alt Parse successful
            XML-->>GT: root element
            
            loop For each test case in XML
                GT->>PE: process_xml_element(test_case)
                activate PE
                
                alt element.attrib exists
                    PE->>PE: Update with attributes
                end
                
                PE->>PE: Convert strings to int/float
                PE-->>GT: case dict
                deactivate PE
                
                GT->>GT: Add workflow_id to case
            end
            
        else Parse failed
            XML-->>GT: Exception
            GT->>GT: Print warning, skip report
        end
        deactivate XML
    end
    
    GT-->>Main: test_cases list
    deactivate GT
    
    Main->>AGG: Aggregate test_cases
    activate AGG
    
    loop For each test_case
        alt Processing successful
            AGG->>AGG: get_key(test_case)
            AGG->>AGG: Initialize if new key
            AGG->>AGG: Increment counters
            AGG->>AGG: Add time (with fallback to 0)
        else Processing failed
            AGG->>AGG: Skip test_case
        end
    end
    
    AGG-->>Main: Aggregated results
    deactivate AGG
    
    Main->>Main: Upload results
Loading

🔗 Cross-Repository Impact Analysis

Enable automatic detection of breaking changes across your dependent repositories. → Set up now

Learn more about Cross-Repository Analysis

What It Does

  • Automatically identifies repositories that depend on this code
  • Analyzes potential breaking changes across your entire codebase
  • Provides risk assessment before merging to prevent cross-repo issues

How to Enable

  1. Visit Settings → Code Management
  2. Configure repository dependencies
  3. Future PRs will automatically include cross-repo impact analysis!

Benefits

  • 🛡️ Prevent breaking changes across repositories
  • 🔍 Catch integration issues before they reach production
  • 📊 Better visibility into your multi-repo architecture

▶️AI Code Reviews for VS Code, Cursor, Windsurf
Install the extension

Note for Windsurf Please change the default marketplace provider to the following in the windsurf settings:

Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery

Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items

Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts below

Emoji Descriptions:

  • ⚠️ Potential Issue - May require further investigation.
  • 🔒 Security Vulnerability - Fix to ensure system safety.
  • 💻 Code Improvement - Suggestions to enhance code quality.
  • 🔨 Refactor Suggestion - Recommendations for restructuring code.
  • ℹ️ Others - General comments and information.

Interact with the Bot:

  • Send a message or request using the format:
    @entelligenceai + *your message*
Example: @entelligenceai Can you suggest improvements for this code?
  • Help the Bot learn by providing feedback on its responses.
    @entelligenceai + *feedback*
Example: @entelligenceai Do not comment on `save_auth` function !

Also you can trigger various commands with the bot by doing
@entelligenceai command

The current supported commands are

  1. config - shows the current config
  2. retrigger_review - retriggers the review

More commands to be added soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant