Log all exceptions out#168
Merged
Merged
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves observability across the app/service/core modules by consistently logging caught exceptions (and replacing a few previously silent/println paths) via the shared com.github.kr328.clash.common.log.Log wrapper.
Changes:
- Add
Log.e(..., e)in multiple catch blocks andrunCatching { ... }.onFailure { ... }branches to preserve stack traces. - Replace
println(e)and some previously ignored exceptions with structured error logs. - Add unexpected-exception logging in the coroutine
ticker()helper (while avoiding logging cancellations).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| service/src/main/kotlin/com/github/kr328/clash/service/clash/module/ConfigurationModule.kt | Log failures when loading an active profile before emitting LoadException. |
| service/src/main/kotlin/com/github/kr328/clash/service/TunService.kt | Log failures when adding allowed/disallowed applications to the VPN builder. |
| service/src/main/kotlin/com/github/kr328/clash/service/ProfileWorker.kt | Log failures during profile update background work. |
| service/src/main/kotlin/com/github/kr328/clash/service/ProfileManager.kt | Replace println(e) with Log.e for update-flow failures. |
| service/src/main/kotlin/com/github/kr328/clash/service/FilesProvider.kt | Log exceptions from document queries while returning empty cursors. |
| core/src/main/kotlin/com/github/kr328/clash/core/Clash.kt | Log override read failures before falling back to defaults. |
| common/src/main/kotlin/com/github/kr328/clash/common/util/Ticker.kt | Log unexpected ticker coroutine exceptions; avoid logging cancellations. |
| app/src/main/kotlin/com/github/kr328/clash/util/Service.kt | Keep unbindServiceSilent truly silent with an empty catch. |
| app/src/main/kotlin/com/github/kr328/clash/util/Application.kt | Log failures during APK ABI verification. |
| app/src/main/kotlin/com/github/kr328/clash/settings/vm/MetaFeatureSettingsViewModel.kt | Log import failures for geo database import flow. |
| app/src/main/kotlin/com/github/kr328/clash/remote/Service.kt | Log failures to bind the remote service. |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/ProvidersViewModel.kt | Log provider update failures before surfacing UI error state. |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/PropertiesViewModel.kt | Log failures for auto-save, release, and commit operations. |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/NewProfileViewModel.kt | Log failures when creating profiles (external/QR/standard). |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/FilesViewModel.kt | Log failures for file operations (list/import/export/rename/delete). |
| app/src/main/kotlin/com/github/kr328/clash/main/vm/MainViewModel.kt | Log failures when starting the Clash/VPN service. |
| app/src/main/kotlin/com/github/kr328/clash/log/vm/LogcatViewModel.kt | Log failures around log export and (un)binding/resume error paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR increases observability by adding structured error logging (via the shared com.github.kr328.clash.common.log.Log wrapper) to previously silent exception paths across app/service/core/common modules.
Changes:
- Add
Log.e(...)calls to manycatch/runCatching().onFailure { ... }blocks that previously ignored exceptions or usedprintln. - Update
ticker()to log unexpected exceptions while avoiding logging coroutine cancellation. - Add error logging around VPN access-control application allow/deny configuration failures.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| service/src/main/kotlin/com/github/kr328/clash/service/clash/module/ConfigurationModule.kt | Log failures when loading the active profile. |
| service/src/main/kotlin/com/github/kr328/clash/service/TunService.kt | Log failures when adding allowed/disallowed applications to the VPN builder. |
| service/src/main/kotlin/com/github/kr328/clash/service/ProfileWorker.kt | Log failures during scheduled/profile update processing. |
| service/src/main/kotlin/com/github/kr328/clash/service/ProfileManager.kt | Replace println(e) with structured error logging in update flow. |
| service/src/main/kotlin/com/github/kr328/clash/service/FilesProvider.kt | Log exceptions during document queries. |
| core/src/main/kotlin/com/github/kr328/clash/core/Clash.kt | Log exceptions when reading overrides and return defaults. |
| common/src/main/kotlin/com/github/kr328/clash/common/util/Ticker.kt | Log unexpected ticker failures and rethrow, while avoiding cancellation logs. |
| app/src/main/kotlin/com/github/kr328/clash/util/Service.kt | Keep unbindServiceSilent() silent (no logging). |
| app/src/main/kotlin/com/github/kr328/clash/util/Application.kt | Log failures during APK ABI verification. |
| app/src/main/kotlin/com/github/kr328/clash/settings/vm/MetaFeatureSettingsViewModel.kt | Log geo database import failures. |
| app/src/main/kotlin/com/github/kr328/clash/remote/Service.kt | Log failures when binding to the remote service. |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/ProvidersViewModel.kt | Log provider update failures. |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/PropertiesViewModel.kt | Log failures during autosave/release/commit operations. |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/NewProfileViewModel.kt | Log failures when creating profiles via different sources. |
| app/src/main/kotlin/com/github/kr328/clash/profile/vm/FilesViewModel.kt | Log failures for file operations (delete/rename/import/export/list). |
| app/src/main/kotlin/com/github/kr328/clash/main/vm/MainViewModel.kt | Log failures when starting the Clash service. |
| app/src/main/kotlin/com/github/kr328/clash/log/vm/LogcatViewModel.kt | Add logging around additional bind/unbind failure paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
105
to
113
| try { | ||
| val name = application.getString(R.string.new_profile) | ||
| val uuid = withProfile { create(type, name) } | ||
| eventState.value = EventState.LaunchProperties(uuid) | ||
| } catch (e: Exception) { | ||
| Log.e("Create profile failed: ${e.message}", e) | ||
| eventState.value = | ||
| EventState.ShowMessage(e.message ?: application.getString(R.string.unknown)) | ||
| } |
Comment on lines
68
to
70
| } catch (e: Exception) { | ||
| Log.e("Load profile failed: ${e.message}", e) | ||
| return enqueueEvent(LoadException(e.message ?: "Unknown")) |
Comment on lines
+92
to
95
| } catch (e: Exception) { | ||
| Log.e("Import geo database failed: ${e.message}", e) | ||
| importResult.value = ImportResult.Failed | ||
| } |
Comment on lines
86
to
90
| } catch (e: Exception) { | ||
| Log.e("Create QR profile failed: ${e.message}", e) | ||
| eventState.value = | ||
| EventState.ShowMessage(e.message ?: application.getString(R.string.unknown)) | ||
| } |
Comment on lines
91
to
97
| viewModelScope.launch { | ||
| try { | ||
| client.renameDocument(file.id, newName) | ||
| } catch (e: Exception) { | ||
| Log.e("Rename file failed: ${e.message}", e) | ||
| eventState.value = EventState.ShowMessage(e.message ?: "Unknown error") | ||
| } |
Comment on lines
+161
to
165
| } catch (e: Exception) { | ||
| Log.e("Start clash service failed: ${e.message}", e) | ||
| eventState.value = | ||
| EventState.ShowMessage(application.getString(R.string.unable_to_start_vpn)) | ||
| } |
Comment on lines
79
to
85
| viewModelScope.launch { | ||
| try { | ||
| client.deleteDocument(file.id) | ||
| } catch (e: Exception) { | ||
| Log.e("Delete file failed: ${e.message}", e) | ||
| eventState.value = EventState.ShowMessage(e.message ?: "Unknown error") | ||
| } |
Comment on lines
207
to
209
| } catch (e: Exception) { | ||
| println(e) | ||
| Log.e("Update profile flow failed: ${e.message}", e) | ||
| } |
Comment on lines
+169
to
+175
| .onFailure { e -> Log.e("Add allowed application $it failed: ${e.message}", e) } | ||
| } | ||
| } | ||
| AccessControlMode.DenySelected -> { | ||
| (store.accessControlPackages - packageName).forEach { | ||
| runCatching { addDisallowedApplication(it) } | ||
| .onFailure { e -> Log.e("Add disallowed application $it failed: ${e.message}", e) } |
Comment on lines
58
to
66
| try { | ||
| val profileName = application.getString(R.string.new_profile) | ||
| val uuid = withProfile { | ||
| create(Profile.Type.External, name ?: profileName, uri.toString()) | ||
| } | ||
| eventState.value = EventState.LaunchProperties(uuid) | ||
| } catch (e: Exception) { | ||
| Log.e("Create external profile failed: ${e.message}", e) | ||
| eventState.value = |
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.
No description provided.