feat(tauri): autoupdate on app resume#2822
Conversation
seanaye
commented
Apr 23, 2026
seanaye
commented
- apply update on app resume
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 0 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR refactors the Tauri bundle updater plugin to auto-apply pending updates on app resume, consolidates error handling using Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seanaye
force-pushed
the
seanaye/feat/update-on-resume
branch
from
1d6bda7 to
aea7ce9
Compare
seanaye
force-pushed
the
seanaye/fix/tauri-deadlock-safari
branch
from
5da236f to
b678ff2
Compare
seanaye
force-pushed
the
seanaye/feat/update-on-resume
branch
from
aea7ce9 to
48bb5c3
Compare
seanaye
force-pushed
the
seanaye/fix/tauri-deadlock-safari
branch
from
b678ff2 to
892b134
Compare
seanaye
force-pushed
the
seanaye/feat/update-on-resume
branch
from
48bb5c3 to
aa1685f
Compare
seanaye
force-pushed
the
seanaye/feat/update-on-resume
branch
from
12ee8c8 to
2298c60
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
js/app/tauri/macro_bundle_updater_plugin/src/inbound/plugin.rs (1)
116-144:⚠️ Potential issue | 🔴 CriticalBug:
apply_completed_updateignoresapply_update's return value and always returnsOk(true).
Service::apply_updatereturnsResult<bool, Report>where theboolsignals whether a completed update was actually applied. That value is discarded by the?operator on line 131-133, and the function unconditionally returnsOk(true)on line 143. This has two user-visible consequences:
- The webview is reloaded on every app resume, even when there is no pending update.
RunEvent::Resumedfires every time the app returns to the foreground (especially on mobile); with this bug, line 141'swindow.location.reload()runs on every foreground, dropping in-flight navigation, ephemeral UI state, and WebSocket connections. This is the opposite of the "apply update on resume" intent.perform_update'sfalse => Err("No pending update")branch at line 152 becomes unreachable — the frontend error previously surfaced when clicking "Update" with no pending bundle will now silently succeed.The
tracing::info!("auto-applied pending bundle update on foreground")log inlib.rswill also fire on every resume, masking whether updates are actually being applied.🔧 Proposed fix
- let mut service = service_state.lock().await; - - service - .apply_update(&cache_dir) - .await - .map_err(|e| e.to_string())?; - - drop(service); - - // Reload to pick up the new bundle. Using location.reload() instead of - // navigating to a new URL preserves WKWebView's cookie store. - if let Some(webview) = app_handle.webview_windows().values().next() { - tracing::info!("Bundle update complete, reloading to pick up new assets"); - let _ = webview.eval("window.location.reload();"); - } - Ok(true) -} + let applied = { + let mut service = service_state.lock().await; + service + .apply_update(&cache_dir) + .await + .map_err(|e| e.to_string())? + }; + + if !applied { + return Ok(false); + } + + // Reload to pick up the new bundle. Using location.reload() instead of + // navigating to a new URL preserves WKWebView's cookie store. + if let Some(webview) = app_handle.webview_windows().values().next() { + tracing::info!("Bundle update complete, reloading to pick up new assets"); + let _ = webview.eval("window.location.reload();"); + } + Ok(true) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/tauri/macro_bundle_updater_plugin/src/inbound/plugin.rs` around lines 116 - 144, The function apply_completed_update currently discards the boolean returned by PluginService::apply_update and always returns Ok(true); change it to capture the returned bool (e.g., let applied = service.apply_update(&cache_dir).await.map_err(|e| e.to_string())?;) and then only trigger the webview reload and return Ok(true) when applied is true, otherwise return Ok(false); ensure you still drop the service lock before evaluating the webview and preserve error mapping to String so perform_update's false branch remains reachable and the reload/log only happen on actual applied updates.rust/cloud-storage/native_app_service/src/outbound.rs (1)
68-83: 🧹 Nitpick | 🔵 TrivialMinor: concurrent cache-miss requests can double-fetch.
Two concurrent callers that observe the cache miss will each execute
compute_checksumand re-download the bundle. Because both compute the same hash, the final cache state is still correct — just wasteful. Consider atokio::sync::OnceCell-per-version or a write-lock-first check pattern if duplicate fetches become expensive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/native_app_service/src/outbound.rs` around lines 68 - 83, get_app_bundle_checksum currently suffers duplicate work when two callers observe a cache miss concurrently; change the caching to avoid double-fetch by using a per-version synchronization primitive (e.g., tokio::sync::OnceCell) or a write-lock-first double-check: instead of only read-locking then computing, on miss acquire the checksum_cache write lock, check again for a hit and if still missing insert or fetch a per-version OnceCell (or similar) and call its get_or_init to run compute_checksum exactly once; update references to checksum_cache and the get_app_bundle_checksum logic to use the per-version OnceCell (or the write-lock-first double-check) so concurrent callers wait for the single compute_checksum result rather than running it twice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/tauri/macro_bundle_updater_plugin/src/domain/service.rs`:
- Around line 335-358: apply_update can race and run twice because it relies on
the worker to reset the status; to make it idempotent, explicitly reset the
status watch to Idle at the end of the successful apply path (before or
immediately after calling self.set_bundle_root / cleanup_old_bundles) so
subsequent concurrent apply_update calls will see Ok(false). Locate apply_update
and set self.status_tx (or the status sender used by the service) to
Ok(UpdateStatus::Idle) after persisting the new bundle root (references:
apply_update, set_bundle_root, cleanup_old_bundles, UpdateStatus::Idle,
self.status_tx) so repeated/resumed invocations short-circuit; alternatively,
you can detect start() returning Err(Full) and return Ok(false) to treat an
in-progress reset as already handled.
In `@rust/cloud-storage/native_app_service/src/outbound.rs`:
- Around line 36-45: Add the "stream" feature to the workspace reqwest
dependency in rust/cloud-storage/Cargo.toml, then refactor compute_checksum to
stream the bundle instead of buffering: in compute_checksum (the async fn
returning Result<String, Report<UpdateErr>>), call
reqwest::get(self.get_app_bundle_path()).await?.error_for_status()? and use
.bytes_stream() to iterate chunks (use futures::StreamExt), feed each chunk into
a Sha256 hasher with hasher.update(&chunk), finalize with hasher.finalize() and
format!("{:x}", sum), and keep the existing .context(UpdateErr::Network) error
mapping; ensure necessary imports (futures::StreamExt, sha2::Sha256,
sha2::Digest) are added and the function signature and error type
(Report<UpdateErr>) remain unchanged.
In `@rust/cloud-storage/tools/native_app_server/src/main.rs`:
- Around line 35-43: The error mapping in get_app_bundle_checksum incorrectly
classifies local I/O failures from sha256_hex as UpdateErr::Network; update the
mapping to either wrap with a more accurate variant (e.g., UpdateErr::Io or
UpdateErr::Filesystem) or avoid adding Network context and return the original
io error via report!(e) so filesystem errors are preserved; locate
get_app_bundle_checksum and the sha256_hex call to change the map_err closure to
use the correct UpdateErr variant or omit the Network context.
---
Outside diff comments:
In `@js/app/tauri/macro_bundle_updater_plugin/src/inbound/plugin.rs`:
- Around line 116-144: The function apply_completed_update currently discards
the boolean returned by PluginService::apply_update and always returns Ok(true);
change it to capture the returned bool (e.g., let applied =
service.apply_update(&cache_dir).await.map_err(|e| e.to_string())?;) and then
only trigger the webview reload and return Ok(true) when applied is true,
otherwise return Ok(false); ensure you still drop the service lock before
evaluating the webview and preserve error mapping to String so perform_update's
false branch remains reachable and the reload/log only happen on actual applied
updates.
In `@rust/cloud-storage/native_app_service/src/outbound.rs`:
- Around line 68-83: get_app_bundle_checksum currently suffers duplicate work
when two callers observe a cache miss concurrently; change the caching to avoid
double-fetch by using a per-version synchronization primitive (e.g.,
tokio::sync::OnceCell) or a write-lock-first double-check: instead of only
read-locking then computing, on miss acquire the checksum_cache write lock,
check again for a hit and if still missing insert or fetch a per-version
OnceCell (or similar) and call its get_or_init to run compute_checksum exactly
once; update references to checksum_cache and the get_app_bundle_checksum logic
to use the per-version OnceCell (or the write-lock-first double-check) so
concurrent callers wait for the single compute_checksum result rather than
running it twice.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d9e86bf-79d4-45a5-91a1-565f78972972
⛔ Files ignored due to path filters (1)
rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (13)
js/app/justfilejs/app/tauri/input_accessory_plugin/src/lib.rsjs/app/tauri/macro_bundle_updater_plugin/src/domain/models.rsjs/app/tauri/macro_bundle_updater_plugin/src/domain/service.rsjs/app/tauri/macro_bundle_updater_plugin/src/inbound/plugin.rsjs/app/tauri/src-tauri/src/lib.rsrust/cloud-storage/native_app_service/Cargo.tomlrust/cloud-storage/native_app_service/src/domain/models.rsrust/cloud-storage/native_app_service/src/domain/ports.rsrust/cloud-storage/native_app_service/src/domain/service.rsrust/cloud-storage/native_app_service/src/outbound.rsrust/cloud-storage/tools/native_app_server/Cargo.tomlrust/cloud-storage/tools/native_app_server/src/main.rs
| async fn compute_checksum(&self) -> Result<String, Report<UpdateErr>> { | ||
| async move { | ||
| let url = self.get_app_bundle_path(); | ||
| let bytes = reqwest::get(url).await?.error_for_status()?.bytes().await?; | ||
| let hash = Sha256::digest(&bytes); | ||
| Result::<_, Report>::Ok(format!("{:x}", hash)) | ||
| } | ||
| .await | ||
| .context(UpdateErr::Network) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
reqwest Response bytes_stream method documentation
💡 Result:
The bytes_stream method on reqwest::Response converts the response body into a stream of Bytes chunks. It requires the stream feature of the reqwest crate to be enabled. Signature: pub fn bytes_stream(self) -> impl Stream<Item = Result> It consumes self (the Response), returning an asynchronous stream that yields Result<Bytes, reqwest::Error> for each chunk of the response body. Example usage: use futures_util::StreamExt; let mut stream = reqwest::get("http://httpbin.org/ip") .await? .bytes_stream; while let Some(item) = stream.next.await { println!("Chunk: {:?}", item?); } This is useful for streaming large responses without loading the entire body into memory. The implementation uses http_body_util::BodyDataStream::new internally, mapping errors appropriately. Source code confirms it's available only with feature = "stream", and the stream is produced from the response's body.
Citations:
- 1: https://docs.rs/reqwest/latest/reqwest/struct.Response.html
- 2: https://docs.rs/reqwest/latest/src/reqwest/async_impl/response.rs.html
🏁 Script executed:
# First, find and read the Cargo.toml for native_app_service
fd -t f Cargo.toml | grep -i native_app_service | head -5Repository: macro-inc/macro
Length of output: 108
🏁 Script executed:
# Also find the exact location of outbound.rs
fd -t f outbound.rs | grep native_app_serviceRepository: macro-inc/macro
Length of output: 113
🏁 Script executed:
# Read the current Cargo.toml to check dependencies and features
cat rust/cloud-storage/native_app_service/Cargo.tomlRepository: macro-inc/macro
Length of output: 714
🏁 Script executed:
# Read the outbound.rs file to see imports and full context
cat -n rust/cloud-storage/native_app_service/src/outbound.rs | head -80Repository: macro-inc/macro
Length of output: 3457
🏁 Script executed:
# Check the workspace Cargo.toml at the root
cat rust/Cargo.toml | grep -A 5 reqwestRepository: macro-inc/macro
Length of output: 107
🏁 Script executed:
# Check if there's a workspace definition in the cloud-storage folder
cat rust/cloud-storage/Cargo.toml | head -30Repository: macro-inc/macro
Length of output: 631
🏁 Script executed:
# Search for futures as a workspace dependency
rg "^futures" Cargo.tomlRepository: macro-inc/macro
Length of output: 110
🏁 Script executed:
# Find the root Cargo.toml
fd -t f Cargo.toml | head -3Repository: macro-inc/macro
Length of output: 161
🏁 Script executed:
# Check the workspace dependencies at the root
head -100 Cargo.toml | tail -50Repository: macro-inc/macro
Length of output: 129
🏁 Script executed:
# Look for reqwest and futures in workspace dependencies
cat Cargo.toml | grep -A 2 "reqwest\|futures"Repository: macro-inc/macro
Length of output: 102
🏁 Script executed:
# The workspace root is in rust/cloud-storage. Let's check that Cargo.toml for workspace.dependencies
cat rust/cloud-storage/Cargo.toml | grep -A 50 "\[workspace.dependencies\]"Repository: macro-inc/macro
Length of output: 1548
🏁 Script executed:
# Also check for reqwest and futures definitions
cat rust/cloud-storage/Cargo.toml | tail -100Repository: macro-inc/macro
Length of output: 2919
Add stream feature to reqwest and refactor compute_checksum to stream the bundle instead of buffering it entirely.
reqwest::Response::bytes() loads the full bundle into memory before hashing. Streaming with bytes_stream() keeps memory bounded and is especially important as bundle sizes grow.
Update the workspace dependency in rust/cloud-storage/Cargo.toml:
reqwest = { version = "0.12.12", features = ["hickory-dns", "json"] }to:
reqwest = { version = "0.12.12", features = ["hickory-dns", "json", "stream"] }Then update the method:
♻️ Streaming fix
async fn compute_checksum(&self) -> Result<String, Report<UpdateErr>> {
async move {
let url = self.get_app_bundle_path();
- let bytes = reqwest::get(url).await?.error_for_status()?.bytes().await?;
- let hash = Sha256::digest(&bytes);
+ use futures::StreamExt;
+ let mut stream = reqwest::get(url).await?.error_for_status()?.bytes_stream();
+ let mut hasher = Sha256::new();
+ while let Some(chunk) = stream.next().await {
+ hasher.update(&chunk?);
+ }
+ let hash = hasher.finalize();
Result::<_, Report>::Ok(format!("{:x}", hash))
}
.await
.context(UpdateErr::Network)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/native_app_service/src/outbound.rs` around lines 36 - 45,
Add the "stream" feature to the workspace reqwest dependency in
rust/cloud-storage/Cargo.toml, then refactor compute_checksum to stream the
bundle instead of buffering: in compute_checksum (the async fn returning
Result<String, Report<UpdateErr>>), call
reqwest::get(self.get_app_bundle_path()).await?.error_for_status()? and use
.bytes_stream() to iterate chunks (use futures::StreamExt), feed each chunk into
a Sha256 hasher with hasher.update(&chunk), finalize with hasher.finalize() and
format!("{:x}", sum), and keep the existing .context(UpdateErr::Network) error
mapping; ensure necessary imports (futures::StreamExt, sha2::Sha256,
sha2::Digest) are added and the function signature and error type
(Report<UpdateErr>) remain unchanged.
| async fn get_app_bundle_checksum( | ||
| &self, | ||
| _version: &semver::Version, | ||
| ) -> Result<String, UpdateErr> { | ||
| Ok(self.checksum.clone()) | ||
| ) -> Result<String, Report<UpdateErr>> { | ||
| sha256_hex(&self.archive_path).await.map_err(|e| { | ||
| tracing::error!(error=?e, path=?self.archive_path, "failed to hash bundle archive"); | ||
| report!(e).context(UpdateErr::Network) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: mislabeling a local I/O error as UpdateErr::Network.
sha256_hex fails on local filesystem errors (e.g., missing app-archive.zip), but those are mapped to UpdateErr::Network. For this dev-only test server it is inconsequential, but a dedicated variant (or keeping the original io::Error via report! without context) would be more accurate and easier to debug. Not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/tools/native_app_server/src/main.rs` around lines 35 - 43,
The error mapping in get_app_bundle_checksum incorrectly classifies local I/O
failures from sha256_hex as UpdateErr::Network; update the mapping to either
wrap with a more accurate variant (e.g., UpdateErr::Io or UpdateErr::Filesystem)
or avoid adding Network context and return the original io error via report!(e)
so filesystem errors are preserved; locate get_app_bundle_checksum and the
sha256_hex call to change the map_err closure to use the correct UpdateErr
variant or omit the Network context.
cf5dea5 to
4e55eed
Compare
1 participant
