Add sentry support #9598
Add sentry support #9598
Conversation
Nikola-3
force-pushed
the
add-sentry-support
branch
from
fc2401a
to
63c6999
Compare
Nikola-3
requested review from
MonicaOlejniczak,
alshdavid,
marcins,
mattcompiles and
JakeLane
| @@ -15,6 +15,7 @@ jobs: | |||
| uses: ./.github/workflows/release.yml | |||
| secrets: inherit | |||
| with: | |||
| profile: canary | |||
There was a problem hiding this comment.
Should this be enabled for dev releases as well? Regardless, it feels like the option should be more generic (i.e. a profiling build) and use the same release command, but pass in a env variable / cli option that you read
scripts/build-native.js
Outdated
| @@ -26,7 +28,8 @@ async function build() { | |||
| console.log(`Building ${pkg}...`); | |||
| await new Promise((resolve, reject) => { | |||
| let args = [ | |||
| (wasm ? 'wasm:' : '') + (release ? 'build-release' : 'build'), | |||
| (wasm ? 'wasm:' : '') + | |||
There was a problem hiding this comment.
can we split this out into an if statement? Nested ternaries are hard to read
There was a problem hiding this comment.
Haha this is on me. Nik said the same thing - I agreed but was feeling lazy.
packages/core/core/src/Parcel.js
Outdated
| } catch (e) { | ||
| // Fallthrough | ||
| // eslint-disable-next-line no-console | ||
| console.warn(e); |
There was a problem hiding this comment.
It's fine to continue startup if sentry fails, but we should log a bit more effectively here. logger is available and we should provide some context to the error
| }; | ||
|
|
||
| let sentry_tags = if let Ok(sentry_tags_raw) = std::env::var("PARCEL_SENTRY_TAGS") { | ||
| println!("{}", sentry_tags_raw); |
| return Ok(()); | ||
| } | ||
|
|
||
| println!("Initialising Sentry in rust..."); |
There was a problem hiding this comment.
ideally this would use log - https://crates.io/crates/log
crates/node-bindings/Cargo.toml
Outdated
| [dependencies] | ||
| napi-derive = "2.12.5" | ||
| parcel-js-swc-core = { path = "../../packages/transformers/js/core" } | ||
| parcel-resolver = { path = "../../packages/utils/node-resolver-rs" } | ||
| dashmap = "5.4.0" | ||
| xxhash-rust = { version = "0.8.2", features = ["xxh3"] } | ||
| sentry = "0.32.2" |
There was a problem hiding this comment.
nit:
if you only use these dependencies for sentry you can make them optional then add them into the canary = [] array to only compile them when this feature is enabled.
you'd first make once_cell = { version = "1.19.0", optional = true } then add to the array; see "optional dependencies" https://doc.rust-lang.org/cargo/reference/features.html#optional-dependencies
Nikola-3
force-pushed
the
add-sentry-support
branch
from
61ba378
to
f2f20dd
Compare
exposed parcel to sentry via env var
… updated pipelines and improved logging of tags
Nikola-3
force-pushed
the
add-sentry-support
branch
from
8c087c7
to
bee02cc
Compare
↪️ Pull Request
Canary builds now release and parcel rust code can be conditionally compiled with support for sentry in rust. This is useful for providing error logging in rust at runtime.
💻 Examples
🚨 Test instructions
✔️ PR Todo