fix: address review findings

This commit is contained in:
Jinjing
2026-05-16 17:54:52 -07:00
parent 03c857100c
commit 558abd63dc
5 changed files with 91 additions and 5 deletions
@@ -87,6 +87,17 @@ describe('CrashReportStore', () => {
await expect(store.markSent(report.id)).resolves.toMatchObject({ status: 'dismissed' })
})
it('persists a submitted dismissed report as sent', async () => {
const { store, filePath } = await createStore()
const report = await store.record(input())
await expect(store.dismiss(report.id)).resolves.toMatchObject({ status: 'dismissed' })
await expect(store.markDismissedSent(report.id)).resolves.toMatchObject({ status: 'sent' })
const reloaded = new CrashReportStore(filePath)
await expect(reloaded.getById(report.id)).resolves.toMatchObject({ status: 'sent' })
})
it('serializes concurrent writes', async () => {
const { store } = await createStore()
+13 -1
View File
@@ -57,6 +57,10 @@ export class CrashReportStore {
return this.transitionPending(id, 'sent')
}
async markDismissedSent(id: string): Promise<CrashReportRecord | null> {
return this.transitionStatus(id, 'dismissed', 'sent')
}
async dismiss(id: string): Promise<CrashReportRecord | null> {
return this.transitionPending(id, 'dismissed')
}
@@ -75,6 +79,14 @@ export class CrashReportStore {
private async transitionPending(
id: string,
status: Exclude<CrashReportStatus, 'pending'>
): Promise<CrashReportRecord | null> {
return this.transitionStatus(id, 'pending', status)
}
private async transitionStatus(
id: string,
from: CrashReportStatus,
status: Exclude<CrashReportStatus, 'pending'>
): Promise<CrashReportRecord | null> {
return this.withWrite(async (reports) => {
let result: CrashReportRecord | null = null
@@ -82,7 +94,7 @@ export class CrashReportStore {
if (report.id !== id) {
return report
}
if (report.status !== 'pending') {
if (report.status !== from) {
result = report
return report
}
+28
View File
@@ -76,6 +76,34 @@ describe('registerCrashReportingHandlers', () => {
)
})
it('submits a dismissed report when the already-open prompt sends it', async () => {
const dismissed = report('dismissed', 'crash-already-dismissed')
const sent = report('sent', dismissed.id)
const markDismissedSent = vi.fn(async () => sent)
registerCrashReportingHandlers({
getLatestPending: vi.fn(async () => null),
getById: vi.fn(async () => dismissed),
dismiss: vi.fn(),
markSent: vi.fn(),
markDismissedSent,
listRecent: vi.fn(async () => []),
record: vi.fn(),
formatDiagnosticText: vi.fn()
} as never)
const result = await handlers.get('crashReports:submit')?.(null, {
reportId: dismissed.id,
githubLogin: null,
githubEmail: null
})
expect(result).toEqual({ ok: true, report: sent })
expect(submitFeedbackMock).toHaveBeenCalledWith(
expect.objectContaining({ feedback: expect.stringContaining('[Crash Report]') })
)
expect(markDismissedSent).toHaveBeenCalledWith(dismissed.id)
})
it('submits through feedback and marks the report sent only after success', async () => {
const latest = report('pending', 'crash-submit-success')
const sent = report('sent', latest.id)
+16 -1
View File
@@ -61,7 +61,11 @@ export function registerCrashReportingHandlers(store: CrashReportStore): void {
if (!report) {
return { ok: false, status: null, error: 'No crash report available.' }
}
if (report.status !== 'pending' || uploadedReportIds.has(report.id)) {
const canSubmitDismissedReport = Boolean(args.reportId && report.status === 'dismissed')
if (
(!canSubmitDismissedReport && report.status !== 'pending') ||
uploadedReportIds.has(report.id)
) {
return {
ok: true,
report: uploadedReportIds.has(report.id) ? { ...report, status: 'sent' } : report
@@ -88,6 +92,17 @@ export function registerCrashReportingHandlers(store: CrashReportStore): void {
return { ...result, report }
}
uploadedReportIds.add(report.id)
if (report.status === 'dismissed') {
try {
// Why: startup prompts are dismissed before the user can send from
// the still-open dialog, so successful uploads must update storage.
const sent = await store.markDismissedSent(report.id)
return { ok: true, report: sent ?? { ...report, status: 'sent' } }
} catch (error) {
console.error('[crash-reporting] Failed to mark dismissed crash report sent:', error)
return { ok: true, report: { ...report, status: 'sent' } }
}
}
try {
const sent = await store.markSent(report.id)
return { ok: true, report: sent ?? { ...report, status: 'sent' } }
@@ -34,7 +34,18 @@ export function CrashReportDialog(): React.JSX.Element {
setLoading(true)
try {
const pending = await window.api.crashReports.getLatestPending()
setReport(pending)
let displayedReport = pending
if (pending && promptIfPresent) {
try {
// Why: startup crash prompts are one-shot. The open dialog keeps the
// report data locally if the user chooses to send immediately.
await window.api.crashReports.dismiss({ reportId: pending.id })
displayedReport = { ...pending, status: 'dismissed' as const }
} catch (error) {
console.error('Failed to dismiss crash report after startup prompt:', error)
}
}
setReport(displayedReport)
if (pending && promptIfPresent) {
setOpen(true)
}
@@ -70,10 +81,15 @@ export function CrashReportDialog(): React.JSX.Element {
toast.success('Crash report copied.')
}
const handleDismiss = async (): Promise<void> => {
const dismissReportIfNeeded = async (): Promise<void> => {
if (report?.status === 'pending') {
await window.api.crashReports.dismiss({ reportId: report.id })
setReport({ ...report, status: 'dismissed' })
}
}
const handleDismiss = async (): Promise<void> => {
await dismissReportIfNeeded()
setOpen(false)
}
@@ -112,7 +128,11 @@ export function CrashReportDialog(): React.JSX.Element {
if (submitting && !nextOpen) {
return
}
setOpen(nextOpen)
if (!nextOpen) {
void dismissReportIfNeeded().finally(() => setOpen(false))
return
}
setOpen(true)
}}
>
<DialogContent className="sm:max-w-xl">