Skip to content

fix: drop empty label from network errors#21350

Closed
criticalAY wants to merge 1 commit into
ankidroid:mainfrom
criticalAY:fix/sync-error-message
Closed

fix: drop empty label from network errors#21350
criticalAY wants to merge 1 commit into
ankidroid:mainfrom
criticalAY:fix/sync-error-message

Conversation

@criticalAY

Copy link
Copy Markdown
Contributor

Note

Assisted-By: Claude Opus 4.8 - analysing the upstream cause and testing the fix

Purpose / Description

When a sync fails with a network error (e.g. AnkiWeb is down or unreachable), the error dialog shows a dangling "Error details:" label with nothing after it. There are simply no details to show, so the bare label looks unfinished. This pr cleans that

Also i have opened an issue upstream:

Fixes

Approach

  • Added stripEmptyErrorDetails() in CoroutineHelpers.kt to drop that trailing label before the message is shown

How Has This Been Tested?

unit testing

Learning (optional, can help others)

Describe the research stage

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

The backend appends its localized line to network errors
even when there's nothing to fill it, leaving a dangling label in the error
dialog. Strip it before showing. The label is read from the translations, so
this works in any language.

Assisted-By: Claude Opus 4.8 (analysing the upstream cause)
@david-allison

david-allison commented Jul 7, 2026

Copy link
Copy Markdown
Member

I feel this should be fixed upstream, unless we're deviating from their behaviour.

If we still want to do it here, we should add a test which fails when upstream have fixed the issue, so we can remove the dead code

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Jul 7, 2026
@criticalAY

Copy link
Copy Markdown
Contributor Author

I would incline with later we can remove this once upstream actually ships it since I am not sure how long it would take

@david-allison david-allison removed the Needs Author Reply Waiting for a reply from the original author label Jul 7, 2026
@david-allison

Copy link
Copy Markdown
Member

I'll leave the question open for Reviewer 2.

If we are continuing, please see if you can detect when this code is no longer necessary via a test

  • The code isn't executed often
  • The user impact of the bug is small
  • It's a fair amount of complexity to fix
  • It's likely hard to determine when upstream has fixed the issue and we can remove the code

@BrayanDSO

Copy link
Copy Markdown
Member

it's less work to send a patch to upstream then consume it here

@criticalAY

Copy link
Copy Markdown
Contributor Author

Sure: ankitects/anki#5120

@BrayanDSO BrayanDSO added the Blocked by dependency Currently blocked by some other dependent / related change label Jul 8, 2026
@criticalAY

Copy link
Copy Markdown
Contributor Author

Closing in favour of the upstream fix(merged):

@criticalAY criticalAY closed this Jul 10, 2026
@criticalAY criticalAY deleted the fix/sync-error-message branch July 10, 2026 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Blocked by dependency Currently blocked by some other dependent / related change Needs Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty error should not show half a sentence

3 participants