Skip to content

fix: share startup error logs as file and add export-to-file option - #2565

Open
i5hi wants to merge 1 commit into
developfrom
fix/startup-error-log-share
Open

fix: share startup error logs as file and add export-to-file option#2565
i5hi wants to merge 1 commit into
developfrom
fix/startup-error-log-share

Conversation

@i5hi

@i5hi i5hi commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

Claude finished @i5hi's task in 3m 15s —— View job


Review — PR #2565: share startup error logs as file + export option

  • Gather context (diff vs origin/develop, read changed files, localization keys)
  • Review for correctness bugs, security issues, and regressions
  • Post inline review comments — not possible: the gh api call to create inline PR comments requires approval in this environment (not in --allowedTools), so findings are posted here with file/line references instead
  • Post summary

The change is sound overall: sharing logs as an attached file instead of raw text is a clear improvement, error snackbars on the init-error screen are now localized instead of hardcoded English, and all four localization keys used (errorSharingLogsMessage, logsExportedMessage, logsExportFailedMessage, logsShareOptionExport) exist in the ARB files. showLogsShareSheet/shareLogsAsText are still used by log_viewer_widget.dart, so no dead code. One privacy issue worth fixing before merge, plus a few minor notes.

1. Privacy: shared temp log files are never deleted

lib/core/widgets/share_logs_bottom_sheet.dart:64-73shareLogsAsFile writes a new timestamped copy of the logs to the temporary directory on every share and never removes it. Two consequences:

  1. Copies accumulate in the app cache, one per share, each with a unique name.
  2. "Delete logs" doesn't actually delete them. log.deleteLogs() only wipes the two canonical log files, so every previously shared copy survives in the cache after the user explicitly deleted their logs. For a Bitcoin wallet, log data silently lingering on disk is worth avoiding.

SharePlus.instance.share() completes when the share sheet is dismissed, so cleanup can happen right after:

final file = File(path);
await file.writeAsString(logs.join('\n'));
try {
  await SharePlus.instance.share(
    ShareParams(files: [XFile(path)], subject: 'bull_logs.tsv'),
  );
} finally {
  if (await file.exists()) await file.delete();
}

(On Android, share_plus serves the receiver from a copy in its own cache subfolder, which it clears on the next share — so deleting the source right after share() returns is safe there too.)

Fix this →

Minor notes

  • Subject/filename mismatchshare_logs_bottom_sheet.dart:71: the share subject is the fixed string 'bull_logs.tsv' while the attached file has the timestamped name from _timestampedLogFileName(). Cosmetic; consider reusing the generated name.
  • iPad popover anchor (pre-existing) — the project targets iPad (TARGETED_DEVICE_FAMILY = "1,2"), and ShareParams is built without sharePositionOrigin, which iPadOS needs to anchor the share popover. This predates this PR (the old text share had the same gap), but since this screen is the crash-recovery path, it's worth verifying the share sheet presents correctly on iPad.
  • Inconsistent share behaviorlib/core/widgets/log_viewer_widget.dart:95-112 still uses the old showLogsShareSheet flow with shareLogsAsText. If share-as-text was the problem being fixed (large logs are unwieldy or get truncated as intent text), the log viewer keeps that behavior. Possibly intentional since it shares filtered logs, but flagging the inconsistency.
  • Removing the context.mounted guard before the share call in share_logs_widget.dart:57-60 is correct — context is no longer used before the await; the guard remains where it matters (before the snackbars).

No security issues found: logs go to the app-private cache/temp directory and are shared via the platform share sheet (FileProvider on Android), and the export path unchanged from before uses the system file-save dialog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant