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

init() crashes in preload script #351

Closed
6 of 7 tasks
Achder opened this issue Jul 14, 2021 · 1 comment
Closed
6 of 7 tasks

init() crashes in preload script #351

Achder opened this issue Jul 14, 2021 · 1 comment

Comments

@Achder
Copy link

Achder commented Jul 14, 2021

Versions + Platform

  • SDK version - @sentry/electron@v2.5.1
  • Electron version - electron@v13.1.6
  • Platform - Windows

Description

I'm currently updating my app to use electron 13 and trying to apply proper context isolation and disabling node integration.

webPreferences: {
	webSecurity: isDevelopment ? false : true,
	nodeIntegration: false,
	contextIsolation: true,
	worldSafeExecuteJavaScript: true,
	spellcheck: false,
	preload: path.resolve(path.join(app.getAppPath(), 'preload.js')),
	defaultFontSize: 14,
}

I know from other issues postet here, that I need to call init() in the main process, my preload script and the isolated renderer.
But my preload script crashes with the following error:

VM75 renderer_init.js:93 TypeError: Cannot read property 'getAppPath' of undefined
    at eval (webpack://***/./node_modules/@sentry/electron/esm/main/normalize.js?:10:58)
    at Module../node_modules/@sentry/electron/esm/main/normalize.js (C:\Development\***\dist\main\preload.js:355:1)
    at __webpack_require__ (C:\Development\***\dist\main\preload.js:5085:33)
    at fn (C:\Development\***\dist\main\preload.js:5244:21)
    at eval (webpack://***/./node_modules/@sentry/electron/esm/main/backend.js?:25:69)
    at Module../node_modules/@sentry/electron/esm/main/backend.js (C:\Development\***\dist\main\preload.js:278:1)
    at __webpack_require__ (C:\Development\***\dist\main\preload.js:5085:33)
    at fn (C:\Development\***\dist\main\preload.js:5244:21)
    at eval (webpack://***/./node_modules/@sentry/electron/esm/main/client.js?:9:66)
    at Module../node_modules/@sentry/electron/esm/main/client.js (C:\Development\***\dist\main\preload.js:289:1)

getAppPath() is only available in the main process.

I'm using webpack as a bundler and selected electron-preload as my target for the preload script:

merge(common, {
	mode: 'development',
	target: 'electron-preload',
	entry: 'common/preload.ts',
	output: {
		filename: 'preload.js',
		chunkFilename: '[name].bundle.js',
		path: path.resolve(__dirname, 'dist/main'),
		globalObject: 'global',
	},
})

In my preload script I simply call:

import { init } from '@sentry/electron'
import { config } from './sentry'
init(config)
@timfish
Copy link
Collaborator

timfish commented Jul 14, 2021

For the preload you need to use electron-renderer webpack target.

This issue is caused by how webpack selects which files to bundle depending on the target.

@timfish timfish closed this as completed Oct 8, 2021
rajivshah3 added a commit to iotaledger/firefly that referenced this issue Oct 13, 2021
Per getsentry/sentry-electron#351 (comment), we should use 'electron-renderer' as the target for the preload scripts in order for Sentry to work correctly
maxwellmattryan added a commit to iotaledger/firefly that referenced this issue Feb 15, 2022
* chore: Add Rust error reporting

* chore: Work on reporting for Electron

* fix: Remove logic for prod vs dev

* Only load Sentry in preload scripts, renderers, and main process

* feat: Add onboarding and settings pages for diagnostic reporting

* fix: Adjust Webpack config for Sentry

Per getsentry/sentry-electron#351 (comment), we should use 'electron-renderer' as the target for the preload scripts in order for Sentry to work correctly

* fix: Remove unneeded dep

* feat: Implement functionality with `settings.json` file

* chore: Improve argument parsing method for preload scripts

* fix: Handle app settings from within node bindings

* fix: Add `rustfmt` changes

* chore: Rewrite
`unsafe` Rust comment

* chore: Cleanup restart logic

* fix: Test and fix Electron-side reporting

* chore: Cleanup code and prep for merge

* fix: Make code for production env ONLY

* fix: Apply `rustfmt` fixes

* fix: missing params callback error handling

* Use rustls instead of native-tls

Since we already use rustls in other places (such as wallet.rs and iota.rs), using the native-tls backend creates additional dependencies

* Revert edits made during testing

* refactor: write more concise Rust code

* feat: improve settings advice for restart

* fix: ensure that locale has sensible fallback text

* update: Sentry reporting improvements (#2116)

* Fix sentrycliignore

* Fix Sentry Webpack config

Enable sourcemap generation

* Get version from package.json

* Fix Sentry config

* Re-export Sentry.captureException

This is necessary for us to manually capture exceptions that are usually captured by our own error handlers

* Call captureException from our error handlers

* chore: add webpack definitions for important environment variables

* fix: adjust logic for determining if we can send diagnostics

* chore: simplify logic for loading Sentry

* fix: adjust loading for `main.js`

Co-authored-by: Matthew Maxwell <maxwellmattryan@gmail.com>
Co-authored-by: Matthew Maxwell <44885822+maxwellmattryan@users.noreply.github.com>

* feat: one-time popup asking users to share diagnostic data

* chore: change phrasing from "send diagnostics" to "send crash reports"

* test: add test errors in some spots for demo-ing

* Build with debug symbols

This provides better stacktraces, but increases the binary size a bit

* Fix formatting

* refactor: use module-exported `captureException` method

* chore: remove text hint from crash reports setting

* fix: Use option_env! to get Sentry DSN at compile-time

* chore: Fix clippy manual_map warning

* feat: Remove PII from Sentry reports (#2243)

* Remove server_name tag from scope

* feat: Pass machine ID to Sentry JS client

* feat: Expose getMachineId via context bridge

* feat: Pass machine ID to Sentry Rust client

* chore: Update lockfile

* fix: Work around node-machine-id require in renderer process

* feat: Remove server_name from Rust events

* fix: Fix passing machine ID to Sentry Rust client

* fix: add parameter to Sentry JS module to avoid always initializing (#2267)

* chore: add text hints back

* chore: slightly change phrasing

* chore: Resolve lint error

* chore: Delete Webpack chunks

* fix: remove JS build artifacts

* fix: change `Platform` to `Electron` for desktop scripts

* chore: update repository for desktop metadata

* fix: remove incorrect platform usage

* fix: only import shared platform interface when in renderer process

* chore: remove test errors from code

* chore: add Sentry support in build workflow

* fix: forgot about `SENTRY_ENVIRONMENT`

* fix: remove platform abstraction from desktop `App.svelte`

* fix: get branch back up to speed with `develop`

* fix: remove small errors in code

* chore: update Sentry dependencies

* chore: cleanup code and remove errors

* chore: cleanup code some more

* feat: change default to `true` in onboarding page for send crash reports

* chore: Resolve lint warning

* fix(ci): Small fix to rebuild of backend actor system

* fix(ci): Fix env var names for Sentry DSNs

Co-authored-by: Rajiv Shah <rajivshah1@icloud.com>
Co-authored-by: Nicole O'Brien <nicole.obrien@iota.org>
amadeu2 pushed a commit to iotaledger/firefly that referenced this issue Feb 20, 2022
* chore: Add Rust error reporting

* chore: Work on reporting for Electron

* fix: Remove logic for prod vs dev

* Only load Sentry in preload scripts, renderers, and main process

* feat: Add onboarding and settings pages for diagnostic reporting

* fix: Adjust Webpack config for Sentry

Per getsentry/sentry-electron#351 (comment), we should use 'electron-renderer' as the target for the preload scripts in order for Sentry to work correctly

* fix: Remove unneeded dep

* feat: Implement functionality with `settings.json` file

* chore: Improve argument parsing method for preload scripts

* fix: Handle app settings from within node bindings

* fix: Add `rustfmt` changes

* chore: Rewrite
`unsafe` Rust comment

* chore: Cleanup restart logic

* fix: Test and fix Electron-side reporting

* chore: Cleanup code and prep for merge

* fix: Make code for production env ONLY

* fix: Apply `rustfmt` fixes

* fix: missing params callback error handling

* Use rustls instead of native-tls

Since we already use rustls in other places (such as wallet.rs and iota.rs), using the native-tls backend creates additional dependencies

* Revert edits made during testing

* refactor: write more concise Rust code

* feat: improve settings advice for restart

* fix: ensure that locale has sensible fallback text

* update: Sentry reporting improvements (#2116)

* Fix sentrycliignore

* Fix Sentry Webpack config

Enable sourcemap generation

* Get version from package.json

* Fix Sentry config

* Re-export Sentry.captureException

This is necessary for us to manually capture exceptions that are usually captured by our own error handlers

* Call captureException from our error handlers

* chore: add webpack definitions for important environment variables

* fix: adjust logic for determining if we can send diagnostics

* chore: simplify logic for loading Sentry

* fix: adjust loading for `main.js`

Co-authored-by: Matthew Maxwell <maxwellmattryan@gmail.com>
Co-authored-by: Matthew Maxwell <44885822+maxwellmattryan@users.noreply.github.com>

* feat: one-time popup asking users to share diagnostic data

* chore: change phrasing from "send diagnostics" to "send crash reports"

* test: add test errors in some spots for demo-ing

* Build with debug symbols

This provides better stacktraces, but increases the binary size a bit

* Fix formatting

* refactor: use module-exported `captureException` method

* chore: remove text hint from crash reports setting

* fix: Use option_env! to get Sentry DSN at compile-time

* chore: Fix clippy manual_map warning

* feat: Remove PII from Sentry reports (#2243)

* Remove server_name tag from scope

* feat: Pass machine ID to Sentry JS client

* feat: Expose getMachineId via context bridge

* feat: Pass machine ID to Sentry Rust client

* chore: Update lockfile

* fix: Work around node-machine-id require in renderer process

* feat: Remove server_name from Rust events

* fix: Fix passing machine ID to Sentry Rust client

* fix: add parameter to Sentry JS module to avoid always initializing (#2267)

* chore: add text hints back

* chore: slightly change phrasing

* chore: Resolve lint error

* chore: Delete Webpack chunks

* fix: remove JS build artifacts

* fix: change `Platform` to `Electron` for desktop scripts

* chore: update repository for desktop metadata

* fix: remove incorrect platform usage

* fix: only import shared platform interface when in renderer process

* chore: remove test errors from code

* chore: add Sentry support in build workflow

* fix: forgot about `SENTRY_ENVIRONMENT`

* fix: remove platform abstraction from desktop `App.svelte`

* fix: get branch back up to speed with `develop`

* fix: remove small errors in code

* chore: update Sentry dependencies

* chore: cleanup code and remove errors

* chore: cleanup code some more

* feat: change default to `true` in onboarding page for send crash reports

* chore: Resolve lint warning

* fix(ci): Small fix to rebuild of backend actor system

* fix(ci): Fix env var names for Sentry DSNs

Co-authored-by: Rajiv Shah <rajivshah1@icloud.com>
Co-authored-by: Nicole O'Brien <nicole.obrien@iota.org>
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

No branches or pull requests

2 participants