[WIP/DNM] Hackweek - PR page #97989
[WIP/DNM] Hackweek - PR page #97989
Conversation
github-actions
bot
added
the
Scope: Frontend
github-actions
bot
added
the
Scope: Backend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return Response(response_data) | ||
|
|
||
| except InvalidSearchQuery as e: | ||
| return Response({"detail": f"Invalid search query: {e}"}, status=400) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 4 hours ago
To fix the problem, we should avoid returning the raw exception message to the user. Instead, return a generic error message such as "Invalid search query." in the response, while logging the actual exception message and stack trace for internal debugging. This change should be made in the except InvalidSearchQuery as e: block, specifically on line 670. No new imports are needed, as logging is already set up. The rest of the code and functionality should remain unchanged.
| @@ -667,7 +667,7 @@ | ||
| "error": str(e), | ||
| }, | ||
| ) | ||
| return Response({"detail": f"Invalid search query: {e}"}, status=400) | ||
| return Response({"detail": "Invalid search query."}, status=400) | ||
| except Exception as e: | ||
| logger.exception( | ||
| "[telkins] Unexpected error in OrganizationPrDataEndpoint", |
❌ 7 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
| }, | ||
| ) | ||
| return Response( | ||
| {"detail": f"An error occurred while processing PR data: {str(e)}"}, status=500 |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 4 hours ago
To fix the problem, we should avoid returning the string representation of the exception (str(e)) in the HTTP response to the user. Instead, return a generic error message such as "An error occurred while processing PR data." The full exception details should continue to be logged server-side for debugging. This change should be made in the except Exception as e: block, specifically on line 680 of src/sentry/issues/endpoints/organization_pr_issues.py. No new imports or methods are required, as logging is already in place.
| @@ -677,7 +677,7 @@ | ||
| }, | ||
| ) | ||
| return Response( | ||
| {"detail": f"An error occurred while processing PR data: {str(e)}"}, status=500 | ||
| {"detail": "An error occurred while processing PR data."}, status=500 | ||
| ) | ||
|
|
||
| def get_projects(self, request: Request, organization: Organization) -> list[Project]: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| request, organization, projects, environments, filenames | ||
| ) | ||
|
|
||
| return Response(issues_data) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 4 hours ago
To fix the problem, we should avoid exposing the raw exception message to the user. Instead, we should return a generic error message in the API response, while still logging the detailed exception (including the stack trace) on the server for debugging purposes. This can be achieved by modifying the _get_issues_data method to return a generic error message in the "error" field, rather than including str(e). Only the log should contain the full exception details. The rest of the code can remain unchanged, as this preserves existing functionality and error handling.
Specifically, in src/sentry/issues/endpoints/organization_pr_issues.py, update the except Exception as e: block in _get_issues_data (lines 461-464) to return a generic error message, e.g., "Failed to fetch issues due to an internal error.", instead of including str(e).
| @@ -460,7 +460,7 @@ | ||
|
|
||
| except Exception as e: | ||
| logger.exception("[telkins] Error getting issues data for PR") | ||
| return {"issues": [], "error": f"Failed to fetch issues: {str(e)}"} | ||
| return {"issues": [], "error": "Failed to fetch issues due to an internal error."} | ||
|
|
||
|
|
||
| @region_silo_endpoint |
| }, | ||
| ) | ||
| return Response( | ||
| {"detail": f"An error occurred while processing PR issues data: {str(e)}"}, |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 4 hours ago
To fix the problem, we should avoid exposing the stringified exception (str(e)) in the user-facing response. Instead, return a generic error message such as "An error occurred while processing PR issues data." to the user. The detailed exception information should continue to be logged using logger.exception for internal debugging. The change should be made in the except Exception as e: block in the get method of the relevant class in src/sentry/issues/endpoints/organization_pr_issues.py, specifically replacing line 206.
No new imports or methods are required, as logging is already in place.
| @@ -203,7 +203,7 @@ | ||
| }, | ||
| ) | ||
| return Response( | ||
| {"detail": f"An error occurred while processing PR issues data: {str(e)}"}, | ||
| {"detail": "An error occurred while processing PR issues data."}, | ||
| status=500, | ||
| ) | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| }, | ||
| ) | ||
| return Response( | ||
| {"detail": f"An error occurred while fetching pull requests: {str(e)}"}, |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 3 hours ago
To fix the problem, we should avoid including the exception message (str(e)) in the response sent to the user. Instead, return a generic error message such as "An error occurred while fetching pull requests." The exception and stack trace should continue to be logged server-side for debugging purposes. The change should be made in the get method of OrganizationPullRequestsEndpoint, specifically in the block handling exceptions (lines 217-228). No new imports are needed, as logging is already set up.
| @@ -223,7 +223,7 @@ | ||
| }, | ||
| ) | ||
| return Response( | ||
| {"detail": f"An error occurred while fetching pull requests: {str(e)}"}, | ||
| {"detail": "An error occurred while fetching pull requests."}, | ||
| status=500, | ||
| ) | ||
|
|
DO NOT MERGE