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

feat: handle errors better #697

Merged
merged 5 commits into from
Nov 24, 2018
Merged
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
73 changes: 60 additions & 13 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { app, dialog } from 'electron'
import { app, dialog, shell } from 'electron'
import { join } from 'path'
import { store, createDaemon } from './utils'
import startupMenubar from './menubar'
Expand All @@ -12,7 +12,45 @@ if (!app.requestSingleInstanceLock()) {
)

// No windows were opened at this time so we don't need to do app.quit()
process.exit(0)
process.exit(1)
}

const issueTemplate = (e) => `Please describe what you were doing when this error happened.

**Specifications**

- **OS**: ${process.platform}
- **IPFS Desktop Version**: ${app.getVersion()}
Copy link
Member

@lidel lidel Nov 22, 2018

Choose a reason for hiding this comment

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

Something of note, app.getVersion returns electron version instead of one from package.json field:

2018-11-22--18-33-09

It seems to be a known issue (electron/electron#7085) when running development version from a subdirectory (./src/) and should be fine in release build (which uses ./), but would be good to fix it in dev in case someone builds it by hand.

Copy link
Member Author

Choose a reason for hiding this comment

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

@lidel building by hand, i.e., npm run build will give you the correct value. Only if you run it directly with npm start you'll be given Electron's version. We could though read package.json file directly. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

@lidel I'm merging this now. We can take a look at that detail later.

- **Electron Version**: ${process.versions.electron}
- **Chrome Version**: ${process.versions.chrome}

**Error**

\`\`\`
${e.stack}
\`\`\`
`

function handleError (e) {
const option = dialog.showMessageBox({
type: 'error',
title: 'IPFS Desktop has shutdown',
message: 'IPFS Desktop has shutdown because of an error. You can restart the app or report the error to the developers, which requires a GitHub account.',
buttons: [
'Close',
'Report the error to the developers',
'Restart the app'
],
cancelId: 0
})

if (option === 1) {
shell.openExternal(`https://github.com/ipfs-shipyard/ipfs-desktop/issues/new?body=${encodeURI(issueTemplate(e))}`)
} else if (option === 2) {
app.relaunch()
}

app.exit(1)
}

async function setupConnection () {
Expand All @@ -31,21 +69,30 @@ async function setupConnection () {
}

async function run () {
await app.whenReady()

// Initial context object
let ctx = {
ipfsd: await setupConnection()
try {
await app.whenReady()
} catch (e) {
dialog.showErrorBox('Electron could not start', e.stack)
app.exit(1)
}

// Initialize windows. These can add properties to context
await startupMenubar(ctx)
try {
// Initial context object
let ctx = {
ipfsd: await setupConnection()
}

// Initialize windows. These can add properties to context
await startupMenubar(ctx)

// Register hooks
await registerHooks(ctx)
// Register hooks
await registerHooks(ctx)

if (!store.get('seenWelcome')) {
// TODO: open WebUI on Welcome screen
if (!store.get('seenWelcome')) {
// TODO: open WebUI on Welcome screen
}
} catch (e) {
handleError(e)
}
}

Expand Down