Skip to content

fix(coding-conventions): Ensure stable component identity for InlineEventAttachment #120032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sentry wants to merge 2 commits into
base: master
Choose a base branch
from
+59 −4
Open

fix(coding-conventions): Ensure stable component identity for InlineEventAttachment #120032

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c700de
fix(coding-conventions): Ensure stable component identity for InlineE…
ryan953 Jul 19, 2026
a5c380f
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 59 additions & 4 deletions static/app/views/issueDetails/groupEventAttachments/inlineEventAttachment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
import styled from '@emotion/styled';

import {getInlineAttachmentRenderer} from 'sentry/components/events/attachmentViewers/previewAttachmentTypes';
import {ImageViewer} from 'sentry/components/events/attachmentViewers/imageViewer';
import {JsonViewer} from 'sentry/components/events/attachmentViewers/jsonViewer';
import {LogFileViewer} from 'sentry/components/events/attachmentViewers/logFileViewer';
import {
getInlineAttachmentRenderer,
imageMimeTypes,
webmMimeTypes,
} from 'sentry/components/events/attachmentViewers/previewAttachmentTypes';
import {RRWebJsonViewer} from 'sentry/components/events/attachmentViewers/rrwebJsonViewer';
import {VideoViewer} from 'sentry/components/events/attachmentViewers/videoViewer';
import {PanelItem} from 'sentry/components/panels/panelItem';
import type {Event} from 'sentry/types/event';
import type {IssueAttachment} from 'sentry/types/group';
Expand All @@ -12,21 +21,67 @@ interface InlineAttachmentsProps {
projectSlug: string;
}

interface AttachmentViewerProps {
attachment: IssueAttachment;
eventId: string;
orgSlug: string;
projectSlug: string;
}

const logFileMimeTypes = [
'text/css',
'text/csv',
'text/html',
'text/javascript',
'text/plain',
];
const jsonMimeTypes = [
'application/json',
'application/ld+json',
'text/json',
'text/x-json',
];
Comment on lines +33 to +43

@sentry sentry Bot Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bug: The logFileMimeTypes and jsonMimeTypes arrays are duplicated, creating a risk of them becoming out of sync with the source, which could lead to rendering failures.
Severity: LOW

Suggested Fix

To prevent future divergence, export the logFileMimeTypes and jsonMimeTypes arrays from previewAttachmentTypes.tsx. Then, import and use these arrays directly in inlineEventAttachment.tsx instead of using locally-defined duplicates.

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:
static/app/views/issueDetails/groupEventAttachments/inlineEventAttachment.tsx#L31-L43

Potential issue: The `logFileMimeTypes` and `jsonMimeTypes` arrays are locally
re-defined in `inlineEventAttachment.tsx`, duplicating the same un-exported arrays from
`previewAttachmentTypes.tsx`. While currently synchronized, this creates a
maintainability issue. If the source arrays in `previewAttachmentTypes.tsx` are updated
in the future to support new MIME types, the duplicated arrays in this component will
not be updated automatically. This divergence would cause the attachment viewer to fail
to render an attachment (returning `null`) even after the initial guard check passes,
because the local MIME type list would be stale.

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


function AttachmentViewer({
attachment,
orgSlug,
projectSlug,
eventId,
}: AttachmentViewerProps) {
const commonProps = {attachment, orgSlug, projectSlug, eventId};

if (imageMimeTypes.includes(attachment.mimetype)) {
return <ImageViewer {...commonProps} />;
}
if (webmMimeTypes.includes(attachment.mimetype)) {
return <VideoViewer {...commonProps} />;
}
if (logFileMimeTypes.includes(attachment.mimetype)) {
return <LogFileViewer {...commonProps} />;
}
if (jsonMimeTypes.includes(attachment.mimetype)) {
if (attachment.name === 'rrweb.json' || attachment.name.startsWith('rrweb-')) {
return <RRWebJsonViewer {...commonProps} />;
}
return <JsonViewer {...commonProps} />;
}
return null;
}

@cursor cursor Bot Jul 19, 2026

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.

Duplicated attachment viewer selection

Medium Severity

AttachmentViewer reimplements the mime-type selection already owned by getInlineAttachmentRenderer, including local copies of logFileMimeTypes and jsonMimeTypes. The copy also omits the nonPreviewableExtensions guard, so the gate and renderer can drift and pick different viewers later.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a5c380f. Configure here.


export function InlineEventAttachment({
attachment,
projectSlug,
eventId,
}: InlineAttachmentsProps) {
const organization = useOrganization();
const AttachmentComponent = getInlineAttachmentRenderer(attachment);

if (!AttachmentComponent) {
if (!getInlineAttachmentRenderer(attachment)) {
return null;
}

return (
<AttachmentPreviewWrapper>
<AttachmentComponent
<AttachmentViewer
orgSlug={organization.slug}
projectSlug={projectSlug}
eventId={eventId}
Expand Down
Loading