Skip to content

ref(sentryapp): Parse buffered request dicts into the RPC model directly - #122370

Draft
mrduncan wants to merge 1 commit into
masterfrom
mrduncan/simplify-app-request-serial
Draft

ref(sentryapp): Parse buffered request dicts into the RPC model directly#122370
mrduncan wants to merge 1 commit into
masterfrom
mrduncan/simplify-app-request-serial

Conversation

@mrduncan

Copy link
Copy Markdown
Member

serialize_rpc_sentry_app_request mapped all ten fields by hand. That was necessary when it took a dict[str, Any], but the input has been the SentryAppRequest TypedDict since the control endpoint landed, and every key already matches the RPC model.

The manual mapping was the one place in the chain that failed silently: a field added to the TypedDict but missed here was dropped with no error, while every other site raises. parse_obj keeps behavior identical, since pydantic gives optional fields an implicit None default and ignores extra keys.

serialize_rpc_sentry_app_request mapped all ten fields by hand. That was necessary when it took a dict[str, Any], but the input has been the SentryAppRequest TypedDict since the control endpoint landed, and every key already matches the RPC model.

The manual mapping was the one place in the chain that failed silently: a field added to the TypedDict but missed here was dropped with no error, while every other site raises. parse_obj keeps behavior identical, since pydantic gives optional fields an implicit None default and ignores extra keys.
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 20, 2026
@mrduncan
mrduncan marked this pull request as ready for review August 20, 2026 20:02
@mrduncan
mrduncan requested review from a team as code owners August 20, 2026 20:02

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a8383f8. Configure here.

request_headers=request.get("request_headers"),
response_body=request.get("response_body"),
)
return RpcSentryAppRequest.parse_obj(request)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing org id fails parse

High Severity

parse_obj does not treat organization_id as optional. On RpcSentryAppRequest that field is int | None with no default, so Pydantic v1 still requires the key. Buffered requests for internal apps omit organization_id, so serialize_rpc_sentry_app_request now raises instead of producing organization_id=None as .get() did. Listing webhook requests for those apps can fail.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a8383f8. Configure here.

request_headers=request.get("request_headers"),
response_body=request.get("response_body"),
)
return RpcSentryAppRequest.parse_obj(request)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The call to RpcSentryAppRequest.parse_obj() will raise a ValidationError for internal Sentry apps because the required organization_id field is intentionally omitted from their request data.
Severity: HIGH

Suggested Fix

Update the RpcSentryAppRequest model to make the organization_id field optional by adding a default value. Change the field definition from organization_id: int | None to organization_id: int | None = None. This will allow parse_obj to successfully process request data that is missing the key, which is the expected behavior for internal apps.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/sentry_apps/services/app_request/serial.py#L6

Potential issue: The change to using `RpcSentryAppRequest.parse_obj(request)` introduces
a runtime validation error for internal Sentry apps. The `organization_id` field is
defined as `int | None` in the `RpcSentryAppRequest` model, which Pydantic v1 treats as
a required field because it lacks a default value. However, for internal apps, the
`organization_id` key is intentionally omitted from the request data dictionary. The
previous code handled this by using `request.get("organization_id")`, which safely
returned `None`. The new code will now raise a `ValidationError` when processing
requests for internal apps, causing the retrieval of their webhook requests to fail.

Did we get this right? 👍 / 👎 to inform future reviews.

@mrduncan
mrduncan marked this pull request as draft August 20, 2026 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant