Repository navigation
fix(sentry): report the real app version as the release, on every head - #149
Merged
Merged
Conversation
Sentry tagged every Android event `release: 0.0.0`, so release health, regression detection and "which build introduced this?" were inert for mobile. The release came from `Assembly.GetEntryAssembly()?.GetName().Version?.ToString() ?? "0.0.0"`, inherited from upstream where it is sound: a WPF app has one head and it is always the entry assembly. It asks a question whose answer is a property of the head, not of the app, and this port has three heads. On Android the process starts through JNI with no managed `Main`, so there is no entry assembly at all and the call fell through to the literal fallback. iOS and desktop each did answer — both with `3.3.0.0`, the right number in the wrong shape (the stores list `3.3.0`) and with no commit attached. Resolve the version from the shared assembly instead — the one every head links in — through a new `Daqifi.Desktop.Common.AppVersion`. It reads `AssemblyInformationalVersionAttribute`, the same attribute the mobile shell's version label already used, so what a user reads off the screen and what a crash is tagged with cannot disagree. The release keeps the `+<sha>` build metadata: it makes each build its own release and maps a crash to an exact commit, which matters on mobile where a store version and a build are not 1:1. Nothing invents a version any more. When it cannot be resolved, `options.Release` is left unset so the SDK applies its own detection; a placeholder like "0.0.0" is indistinguishable in Sentry from a release that genuinely is 0.0.0, which is how this stayed invisible until someone read a crash report closely. Also single-source the version number itself. "3.3.0" was spelled out in four projects — the shared library, the Desktop head, and both store display versions — and drift between the assembly Sentry reports and the version a store lists is the same class of bug. They now all read `DaqifiAppVersion` from Directory.Build.props. The per-store build numbers stay per-store. Closes #126
Contributor
Author
|
/agentic_review |
PR Summary by QodoReport accurate Sentry releases across all app heads
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Not merging — for review.
Closes #126.
What was wrong
Every crash and error the Android app sent to Sentry arrived tagged
release: 0.0.0. That is not a version this app has ever had, so release health, regression detection and "which build introduced this?" were all inert for mobile: crashes from every build in the wild landed in one undifferentiated bucket. (diststill carried the Android versionCode, so triage was degraded rather than blind — which is why this wasn't a ship blocker.)The release string came from a line inherited from the Windows app:
That is sound upstream, where there is exactly one head and it is always the entry assembly. It asks a question whose answer is a property of the head, not of the app — and this port has three heads:
GetEntryAssembly()?.GetName().VersionMain), soGetEntryAssembly()isnull0.0.0— the literal fallback<Version>declared, but the Apple SDK derives it from<ApplicationDisplayVersion>3.3.0.0<Version>3.3.0.0So Android was broken and the other two were merely lucky — right number, wrong shape (the stores list
3.3.0, not3.3.0.0), no commit attached, and resting on which assembly happens to be the entry point.I checked the iOS head rather than assuming, and the issue's guess that it would report
1.0.0.0turns out to be wrong:dotnet msbuild Daqifi.Avalonia.iOS.csproj -getProperty:Versionevaluates to3.3.0even though the csproj sets no<Version>, because the Apple SDK mapsApplicationDisplayVersiononto it.The fix
Ask the shared assembly — the one every head links in — instead of asking whichever assembly started the process.
New
Daqifi.Desktop.Common.AppVersionreadsAssemblyInformationalVersionAttribute, which is the same attribute the mobile shell's version label already used. The Sentry init and the on-screen version now come from one resolver, so what a user reads off the screen and what a crash is tagged with cannot disagree.The release keeps the
+<sha>build metadata (3.3.0+d7cfaff…) rather than being trimmed to3.3.0. This is the deliberate choice the issue asked for: it makes every build its own release and maps a crash to an exact commit, which is what you want on mobile where a store version and a build are not 1:1. The user-facing label staysv3.3.0— the SHA is dev noise on screen (#13).Nothing invents a version any more. When the version cannot be resolved,
options.Releaseis left unset so the Sentry SDK applies its own detection. A stand-in like"0.0.0"is indistinguishable in Sentry from a release that genuinely is0.0.0, which is exactly how this stayed invisible until someone read a crash report closely.Also: one place to bump the version
3.3.0was hand-written in four projects — the shared library, the Desktop head, and the Android and iOS store display versions. Drift between the assembly Sentry reports and the version a store lists is the same class of bug as the one above, with the same symptom: a triage conversation starting from a false premise. All four now readDaqifiAppVersionfromDirectory.Build.props; bumping that one property cuts a release.The per-store build numbers (
ApplicationVersion— Android versionCode, iOSCFBundleVersion) are deliberately not folded in. They are per-store counters on their own schedule, and Play requires a unique increasing code for every upload including re-uploads of an unchanged marketing version.Verification
There were no guarding tests to strengthen first — this repo has no test project at all. That is the honest finding, and it is why the verification below is a build-artifact read-back rather than a test.
The check was a throwaway console that loads the built
Daqifi.Avalonia.dlland asksAppVersionfor the strings the Sentry init consumes (reflection only, so nothing needs Avalonia loaded). It lived outside the repo and is not part of this change — the diff adds no test scaffolding.Against the trimmed iOS Release assembly as it sits inside the built
.app— the toughest case, since the iOS Release build trims and AOT-compiles, and a stripped attribute would silently reintroduce the bug:d7cfaff…is this branch's head commit exactly. The same read-back against the desktop build output gives the same answer. The attribute survives the trimmer, and the release string is now identical on both heads I can build.Also confirmed the MSBuild single-sourcing actually reached the heads: the built iOS bundle's
Info.plistcarriesCFBundleShortVersionString = 3.3.0(from$(DaqifiAppVersion)) withCFBundleVersion = 1unchanged.Build gate, all green, clean rebuilds:
-r;git status -- '**/packages.lock.json'empty, no lock churn)iossimulator-arm64On the iOS number: I measured
origin/mainin the same worktree under the identical clean-rebuild conditions and it is also 1253, so this change adds zero warnings. The 1249 figure in the loop brief appears to be a slightly different measurement state; the like-for-like comparison is the meaningful one, and 1233 is what an incremental build reports because the vendored OxyPlot's 20 warnings are not re-emitted.What I did not verify
<ApplicationDisplayVersion>3.3.0</ApplicationDisplayVersion>→$(DaqifiAppVersion), which MSBuild evaluation confirms resolves to the same3.3.0. I did not produce an Android build myself — CI did, and the Android head job passes on this branch (5m11s), along with every other check. What CI proves is that it compiles and packages; it does not exercise the release string at runtime.releasetag; what I can show is the string the SDK will be handed, read out of the shipped assembly. Closing the loop needs a device run.Left alone deliberately
MainWindow.axaml.csalso resolves a version, viaAssembly.GetExecutingAssembly().GetName().Version.GetExecutingAssembly()there is the shared assembly, so it was never affected by this bug and already showsDAQiFi v3.3.0. It is an upstream-paired file carrying@portmarkers, and churning it for cosmetics would create a divergence to maintain for no behavioural gain.The divergence from upstream's line, and the new downstream-only type, are recorded in
.portomatic/map/Common.yaml(DIV-COM-001).🤖 Generated with Claude Code