Skip to content

Commit bb2bf5d

Browse files
leeandherclaude
andcommitted
fix(integrations): Handle null items in organization integrations serialization
The OrganizationIntegrationSerializer could produce null entries in API responses when per-item serialization failed. The _serialize catch-all in base.py swallowed exceptions and returned None, which was included directly in the response list, crashing the frontend. - Use .get() in get_attrs to handle missing Integration records gracefully instead of KeyError-ing the entire request - Add null guards for missing integration and failed inner serialization - Catch broad exceptions in get_installation and installation methods with specific log messages instead of relying on the generic _serialize catch-all - Filter null items from both organization-integrations endpoints Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b7608da commit bb2bf5d

File tree

4 files changed

+54
-5
lines changed

4 files changed

+54
-5
lines changed

src/sentry/api/endpoints/user_organizationintegrations.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@ def get(self, request: Request, user) -> Response:
5353
return self.paginate(
5454
request=request,
5555
queryset=queryset,
56-
on_results=lambda x: serialize(x, request.user),
56+
on_results=lambda x: [item for item in serialize(x, request.user) if item is not None],
5757
paginator_cls=OffsetPaginator,
5858
)

src/sentry/integrations/api/endpoints/organization_integrations_index.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,11 @@ def get(
125125
def on_results(results: Sequence[OrganizationIntegration]) -> Sequence[Mapping[str, Any]]:
126126
if feature_filters:
127127
results = filter_by_features(results, feature_filters)
128-
return serialize(results, request.user, include_config=include_config)
128+
return [
129+
item
130+
for item in serialize(results, request.user, include_config=include_config)
131+
if item is not None
132+
]
129133

130134
return self.paginate(
131135
queryset=queryset,

src/sentry/integrations/api/serializers/models/integration.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def get_attrs(
135135
)
136136
integrations_by_id: dict[int, RpcIntegration] = {i.id: i for i in integrations}
137137
return {
138-
item: {"integration": integrations_by_id[item.integration_id]} for item in item_list
138+
item: {"integration": integrations_by_id.get(item.integration_id)} for item in item_list
139139
}
140140

141141
def serialize(
@@ -145,27 +145,40 @@ def serialize(
145145
user: User | RpcUser | AnonymousUser,
146146
include_config: bool = True,
147147
**kwargs: Any,
148-
) -> MutableMapping[str, Any]:
148+
) -> MutableMapping[str, Any] | None:
149+
logging_name = "sentry.serializers.model.organizationintegration"
150+
logging_ctx = {"organization_integration_id": obj.id, "integration_id": obj.integration_id}
149151
# XXX(epurkhiser): This is O(n) for integrations, especially since
150152
# we're using the IntegrationConfigSerializer which pulls in the
151153
# integration installation config object which very well may be making
152154
# API request for config options.
153-
integration: RpcIntegration = attrs.get("integration") # type: ignore[assignment]
155+
integration: RpcIntegration | None = attrs.get("integration")
156+
if integration is None:
157+
logger.warning(f"{logging_name}.missing_integration", extra=logging_ctx)
158+
return None
159+
154160
serialized_integration: MutableMapping[str, Any] = serialize(
155161
objects=integration,
156162
user=user,
157163
serializer=IntegrationConfigSerializer(obj.organization_id, params=self.params),
158164
include_config=include_config,
159165
)
166+
if serialized_integration is None:
167+
logger.warning(f"{logging_name}.serialize_integration_failed", extra=logging_ctx)
168+
return None
160169

161170
dynamic_display_information = None
162171
config_data = None
172+
logging_ctx["provider"] = integration.provider
163173

164174
try:
165175
installation = integration.get_installation(organization_id=obj.organization_id)
166176
except NotImplementedError:
167177
# slack doesn't have an installation implementation
168178
config_data = obj.config if include_config else None
179+
except Exception:
180+
logger.exception(f"{logging_name}.get_installation_failed", extra=logging_ctx)
181+
config_data = obj.config if include_config else None
169182
else:
170183
try:
171184
installation.org_integration = obj
@@ -183,6 +196,8 @@ def serialize(
183196
"integration_provider": integration.provider,
184197
}
185198
logger.info(name, extra=log_info)
199+
except Exception:
200+
logger.exception(f"{logging_name}.serialize_installation_failed", extra=logging_ctx)
186201

187202
serialized_integration.update(
188203
{

tests/sentry/integrations/api/endpoints/test_user_organizationintegration.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import orjson
44

5+
from sentry.integrations.models.integration import Integration
56
from sentry.testutils.cases import APITestCase
67
from sentry.testutils.silo import control_silo_test
78

@@ -72,3 +73,32 @@ def test_billing_users_dont_see_integrations(self) -> None:
7273
assert response.status_code == 200
7374
content = orjson.loads(response.content)
7475
assert content == []
76+
77+
def test_missing_integration_filtered_from_response(self) -> None:
78+
"""OrganizationIntegrations whose Integration was deleted don't produce null entries."""
79+
integration = self.create_provider_integration(provider="github")
80+
self.create_organization_integration(
81+
organization_id=self.organization.id, integration_id=integration.id
82+
)
83+
84+
# Delete the Integration row directly to simulate orphaned state
85+
Integration.objects.filter(id=integration.id).delete()
86+
87+
response = self.get_success_response(self.user.id)
88+
assert response.data == []
89+
90+
def test_installation_error_still_returns_item(self) -> None:
91+
"""An exception in get_dynamic_display_information shouldn't null out the item."""
92+
integration = self.create_provider_integration(provider="github")
93+
self.create_organization_integration(
94+
organization_id=self.organization.id, integration_id=integration.id
95+
)
96+
97+
with patch(
98+
"sentry.integrations.base.IntegrationInstallation.get_dynamic_display_information",
99+
side_effect=RuntimeError("boom"),
100+
):
101+
response = self.get_success_response(self.user.id)
102+
103+
assert len(response.data) == 1
104+
assert response.data[0]["organizationId"] == self.organization.id

0 commit comments

Comments
 (0)