-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
feat(issues-stacktrace): Add copy stacktrace button to ellipsis menu #82107
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,18 @@ import {Fragment} from 'react'; | |
import styled from '@emotion/styled'; | ||
|
||
import {openNavigateToExternalLinkModal} from 'sentry/actionCreators/modal'; | ||
import type {Client} from 'sentry/api'; | ||
import {LinkButton} from 'sentry/components/button'; | ||
import ClippedBox from 'sentry/components/clippedBox'; | ||
import rawContent from 'sentry/components/events/interfaces/crashContent/stackTrace/rawContent'; | ||
import ExternalLink from 'sentry/components/links/externalLink'; | ||
import {IconOpen} from 'sentry/icons'; | ||
import LoadingError from 'sentry/components/loadingError'; | ||
import LoadingIndicator from 'sentry/components/loadingIndicator'; | ||
import {IconOpen} from 'sentry/icons/iconOpen'; | ||
import {t} from 'sentry/locale'; | ||
import type {Frame} from 'sentry/types/event'; | ||
import type {Organization} from 'sentry/types/organization'; | ||
import type {PlatformKey} from 'sentry/types/project'; | ||
import {getFileExtension} from 'sentry/utils/fileExtension'; | ||
import {isUrl} from 'sentry/utils/string/isUrl'; | ||
import {safeURL} from 'sentry/utils/url/safeURL'; | ||
|
@@ -33,6 +42,90 @@ interface RenderLinksInTextProps { | |
exceptionText: string; | ||
} | ||
|
||
export const getAppleCrashReportEndpoint = ( | ||
organization: Organization, | ||
type: 'original' | 'minified', | ||
projectSlug: string, | ||
eventId: string | ||
) => { | ||
const minified = type === 'minified'; | ||
return `/projects/${organization.slug}/${projectSlug}/events/${eventId}/apple-crash-report?minified=${minified}`; | ||
}; | ||
|
||
export const getContent = ( | ||
isNative: boolean, | ||
exc: any, | ||
type: 'original' | 'minified', | ||
projectSlug: string, | ||
eventId: string, | ||
api: Client, | ||
platform?: PlatformKey, | ||
loading?: boolean, | ||
error?: boolean, | ||
crashReport?: string, | ||
organization?: Organization | ||
) => { | ||
Comment on lines
+55
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's probably something else we can do here. looks like we wanted both a component and to use the output |
||
const output = { | ||
downloadButton: null, | ||
content: exc.stacktrace | ||
? rawContent( | ||
type === 'original' ? exc.stacktrace : exc.rawStacktrace, | ||
platform, | ||
exc | ||
) | ||
: null, | ||
}; | ||
|
||
if (!isNative) { | ||
return output; | ||
} | ||
|
||
if (loading) { | ||
return { | ||
...output, | ||
content: <LoadingIndicator />, | ||
}; | ||
} | ||
|
||
if (error) { | ||
return { | ||
...output, | ||
content: <LoadingError />, | ||
}; | ||
} | ||
|
||
if (!loading && !!crashReport) { | ||
let downloadButton: React.ReactElement | null = null; | ||
|
||
if (organization) { | ||
const appleCrashReportEndpoint = getAppleCrashReportEndpoint( | ||
organization, | ||
type, | ||
projectSlug, | ||
eventId | ||
); | ||
|
||
downloadButton = ( | ||
<DownloadBtnWrapper> | ||
<LinkButton | ||
size="xs" | ||
href={`${api.baseUrl}${appleCrashReportEndpoint}&download=1`} | ||
> | ||
{t('Download')} | ||
</LinkButton> | ||
</DownloadBtnWrapper> | ||
); | ||
} | ||
|
||
return { | ||
downloadButton, | ||
content: <ClippedBox clipHeight={250}>{crashReport}</ClippedBox>, | ||
}; | ||
} | ||
|
||
return output; | ||
}; | ||
|
||
export const renderLinksInText = ({ | ||
exceptionText, | ||
}: RenderLinksInTextProps): ReactElement => { | ||
|
@@ -108,3 +201,9 @@ const IconPlacement = styled(IconOpen)` | |
margin-left: 5px; | ||
vertical-align: center; | ||
`; | ||
|
||
const DownloadBtnWrapper = styled('div')` | ||
display: flex; | ||
align-items: center; | ||
justify-content: flex-end; | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,16 @@ import {Fragment} from 'react'; | |
|
||
import {CommitRow} from 'sentry/components/commitRow'; | ||
import ErrorBoundary from 'sentry/components/errorBoundary'; | ||
import {getContent} from 'sentry/components/events/interfaces/crashContent/exception/utils'; | ||
import {SuspectCommits} from 'sentry/components/events/suspectCommits'; | ||
import {t} from 'sentry/locale'; | ||
import type {Event, ExceptionType} from 'sentry/types/event'; | ||
import {EntryType} from 'sentry/types/event'; | ||
import type {Group} from 'sentry/types/group'; | ||
import type {Project} from 'sentry/types/project'; | ||
import {StackType, StackView} from 'sentry/types/stacktrace'; | ||
import useApi from 'sentry/utils/useApi'; | ||
import useOrganization from 'sentry/utils/useOrganization'; | ||
import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils'; | ||
|
||
import {TraceEventDataSection} from '../traceEventDataSection'; | ||
|
@@ -35,6 +38,12 @@ export function Exception({ | |
}: Props) { | ||
const eventHasThreads = !!event.entries.some(entry => entry.type === EntryType.THREADS); | ||
const hasStreamlinedUI = useHasStreamlinedUI(); | ||
const api = useApi(); | ||
const organization = useOrganization(); | ||
const isNative = | ||
event.platform === 'cocoa' || | ||
event.platform === 'native' || | ||
event.platform === 'nintendo-switch'; | ||
|
||
// in case there are threads in the event data, we don't render the | ||
// exception block. Instead the exception is contained within the | ||
|
@@ -51,6 +60,28 @@ export function Exception({ | |
|
||
const stackTraceNotFound = !(data.values ?? []).length; | ||
|
||
const copyableStackTrace = | ||
data.values && !isNative | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. native platforms appears to have a threaded stacktraces a lot of the time. If that's the case, this component is not even used. For the sake of simplicity, I've just excluded native platforms from this feature - implementing it for native will be significantly more complex since we may need to fetch crash reports (at least for Cocoa). |
||
? data.values | ||
.map((exc, _excIdx) => { | ||
const {content} = getContent( | ||
false, | ||
exc, | ||
'original', | ||
projectSlug, | ||
event.id, | ||
api, | ||
event.platform, | ||
false, | ||
false, | ||
'', | ||
organization | ||
Comment on lines
+67
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is this madness There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the function to create the raw stacktrace string, and yes, it is insanely ugly and complex for some reason – I extracted this out of |
||
); | ||
return content; | ||
}) | ||
.join('\n\n') | ||
: undefined; | ||
|
||
return ( | ||
<TraceEventDataSection | ||
title={t('Stack Trace')} | ||
|
@@ -91,6 +122,7 @@ export function Exception({ | |
!!data.values?.some(value => (value.stacktrace?.frames ?? []).length > 1) | ||
} | ||
stackTraceNotFound={stackTraceNotFound} | ||
copyableStackTrace={copyableStackTrace} | ||
> | ||
{({recentFirst, display, fullStackTrace}) => { | ||
return stackTraceNotFound ? ( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is responsible for actually wrangling the stacktrace into the stacktrace string that's displayed when "Raw stacktrace" is selected. I have moved two methods,
getContent
andgetAppleCrashreportEndpoint
to theutils.tsx
file to make for better reusability.