Skip to content
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

Add retry method to share link dialog in case of error #4276

Merged
merged 5 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 32 additions & 12 deletions packages/core/ui/ErrorMessage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import React, { Suspense, lazy, useState } from 'react'
import { Button } from '@mui/material'
import { IconButton, Tooltip } from '@mui/material'

// locals
import RedErrorMessageBox from './RedErrorMessageBox'

// icons
import RefreshIcon from '@mui/icons-material/Refresh'
import ReportIcon from '@mui/icons-material/Report'

// lazies
const ErrorMessageStackTraceDialog = lazy(
() => import('./ErrorMessageStackTraceDialog'),
)
Expand Down Expand Up @@ -34,23 +41,36 @@ function parseError(str: string) {
return snapshotError
}

const ErrorMessage = ({ error }: { error: unknown }) => {
const ErrorMessage = ({
error,
onReset,
}: {
error: unknown
onReset?: () => void
}) => {
const str = `${error}`
const snapshotError = parseError(str)
const [showStack, setShowStack] = useState(false)
return (
<RedErrorMessageBox>
{str.slice(0, 10000)}

{typeof error === 'object' && error && 'stack' in error ? (
<Button
style={{ float: 'right' }}
variant="contained"
onClick={() => setShowStack(!showStack)}
>
{showStack ? 'Hide stack trace' : 'Show stack trace'}
</Button>
) : null}
<div style={{ float: 'right', marginLeft: 100 }}>
{typeof error === 'object' && error && 'stack' in error ? (
<Tooltip title="Get stack trace">
<IconButton onClick={() => setShowStack(true)} color="primary">
<ReportIcon />
</IconButton>
</Tooltip>
) : null}
{onReset ? (
<Tooltip title="Retry">
<IconButton onClick={onReset} color="primary">
<RefreshIcon />
</IconButton>
</Tooltip>
) : null}
</div>
{snapshotError ? (
<pre
style={{
Expand All @@ -65,7 +85,7 @@ const ErrorMessage = ({ error }: { error: unknown }) => {
{showStack ? (
<Suspense fallback={null}>
<ErrorMessageStackTraceDialog
error={error as Error}
error={error}
onClose={() => setShowStack(false)}
/>
</Suspense>
Expand Down
58 changes: 41 additions & 17 deletions packages/core/ui/ErrorMessageStackTraceDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,62 @@ import {
Typography,
} from '@mui/material'

import { RawSourceMap, SourceMapConsumer } from 'source-map-js'
import { SourceMapConsumer } from 'source-map-js'
import copy from 'copy-to-clipboard'

// locals
import Dialog from './Dialog'
import LoadingEllipses from './LoadingEllipses'

function Link2({
href,
children,
}: {
href: string
children: React.ReactNode
}) {
return (
<Link target="_blank" href={href}>
{children}
</Link>
)
}

async function myfetch(uri: string) {
const res = await fetch(uri)
if (!res.ok) {
throw new Error(`HTTP ${res.status} fetching ${uri}: ${await res.text()}`)
}
return res
}

async function myfetchjson(uri: string) {
const res = await myfetch(uri)
return res.json()
}

async function myfetchtext(uri: string) {
const res = await myfetch(uri)
return res.text()
}

// produce a source-map resolved stack trace
// reference code https://stackoverflow.com/a/77158517/2129219
const sourceMaps: Record<string, RawSourceMap> = {}
const sourceMaps: Record<string, SourceMapConsumer> = {}
async function getSourceMapFromUri(uri: string) {
if (sourceMaps[uri] != undefined) {
return sourceMaps[uri]
}
const uriQuery = new URL(uri).search
const currentScriptContent = await (await fetch(uri)).text()
const currentScriptContent = await myfetchtext(uri)

let mapUri =
new RegExp(/\/\/# sourceMappingURL=(.*)/).exec(currentScriptContent)?.[1] ||
''
mapUri = new URL(mapUri, uri).href + uriQuery

const map = await (await fetch(mapUri)).json()

const map = new SourceMapConsumer(await myfetchjson(mapUri))
Copy link
Collaborator Author

@cmdcolin cmdcolin Mar 8, 2024

Choose a reason for hiding this comment

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

this is a bonus optimization. if the stack trace goes through 20 stack frames of mobx-state-tree function calls, then it was reparsing the stack trace 20 times which added up to 5 seconds of waiting for the stack trace. this parses it just once

sourceMaps[uri] = map

return map
}

Expand All @@ -48,7 +78,7 @@ async function mapStackTrace(stack: string) {
}

const uri = match[2]
const consumer = new SourceMapConsumer(await getSourceMapFromUri(uri))
const consumer = await getSourceMapFromUri(uri)

const originalPosition = consumer.originalPositionFor({
line: parseInt(match[3]),
Expand Down Expand Up @@ -100,14 +130,8 @@ function Contents({ text }: { text: string }) {
return (
<>
<Typography>
Post a new issue at{' '}
<Link href={githubLink} target="_blank">
GitHub
</Link>{' '}
or send an email to{' '}
<Link href={emailLink} target="_blank">
[email protected]
</Link>{' '}
Post a new issue at <Link2 href={githubLink}>GitHub</Link2> or send an
email to <Link2 href={emailLink}>[email protected]</Link2>{' '}
</Typography>
<pre
style={{
Expand All @@ -129,12 +153,12 @@ export default function ErrorMessageStackTraceDialog({
onClose,
}: {
onClose: () => void
error: Error
error: unknown
}) {
const [mappedStackTrace, setMappedStackTrace] = useState<string>()
const [secondaryError, setSecondaryError] = useState<unknown>()
const [clicked, setClicked] = useState(false)
const stackTracePreProcessed = `${error.stack}`
const stackTracePreProcessed = `${typeof error === 'object' && error !== null && 'stack' in error ? error.stack : ''}`
const errorText = `${error}`
const stackTrace = stripMessage(stackTracePreProcessed, errorText)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function makeSubpartsFilter(
}

return (feature: Feature) =>
(filter as string[])
filter
.map(typeName => typeName.toLowerCase())
.includes(feature.get('type').toLowerCase())
}
Expand Down
40 changes: 18 additions & 22 deletions products/jbrowse-web/src/components/ShareDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,47 +67,43 @@ const ShareDialog = observer(function ({
const snap = getSnapshot(session)

useEffect(() => {
let cancelled = false
// eslint-disable-next-line @typescript-eslint/no-floating-promises
;(async () => {
// checking !error allows retry when error state is cleared
if (error) {
return
}
try {
if (currentSetting === 'short') {
setLoading(true)
const locationUrl = new URL(window.location.href)
const result = await shareSessionToDynamo(snap, url, locationUrl.href)
if (!cancelled) {
const params = new URLSearchParams(locationUrl.search)
params.set('session', `share-${result.json.sessionId}`)
params.set('password', result.password)
locationUrl.search = params.toString()
setShortUrl(locationUrl.href)

setSessionParam(`share-${result.json.sessionId}`)
setPasswordParam(result.password)
}
const params = new URLSearchParams(locationUrl.search)
params.set('session', `share-${result.json.sessionId}`)
params.set('password', result.password)
locationUrl.search = params.toString()
setShortUrl(locationUrl.href)

setSessionParam(`share-${result.json.sessionId}`)
setPasswordParam(result.password)
} else {
const sess = await toUrlSafeB64(JSON.stringify(getSnapshot(session)))
const longUrl = new URL(window.location.href)
const longParams = new URLSearchParams(longUrl.search)
longParams.set('session', `encoded-${sess}`)
setSessionParam(`encoded-${sess}`)
longUrl.search = longParams.toString()
if (!cancelled) {
setLongUrl(longUrl.toString())
}
setLongUrl(longUrl.toString())
}
} catch (e) {
setError(e)
} finally {
setLoading(false)
}
})()
}, [currentSetting, error, session, url, snap])

return () => {
cancelled = true
}
}, [currentSetting, session, url, snap])

const disabled = (currentSetting === 'short' && loading) || !!error
return (
<>
<Dialog
Expand All @@ -126,7 +122,7 @@ const ShareDialog = observer(function ({

{currentSetting === 'short' ? (
error ? (
<ErrorMessage error={error} />
<ErrorMessage error={error} onReset={() => setError(undefined)} />
) : loading ? (
<Typography>Generating short URL...</Typography>
) : (
Expand All @@ -139,7 +135,7 @@ const ShareDialog = observer(function ({
<DialogActions>
<Button
startIcon={<BookmarkAddIcon />}
disabled={currentSetting === 'short' && loading}
disabled={disabled}
onClick={event => {
event.preventDefault()
setPassword(passwordParam, 'replaceIn')
Expand All @@ -156,7 +152,7 @@ const ShareDialog = observer(function ({
session.notify('Copied to clipboard', 'success')
}}
startIcon={<ContentCopyIcon />}
disabled={currentSetting === 'short' && loading}
disabled={disabled}
>
Copy to Clipboard
</Button>
Expand Down
Loading
Loading