Skip to content

Draft: Bug 42479: Improve TorConnect error handling

Merge Info

Related Issues

Backporting

Timeline

  • Immediate: patchset needed as soon as possible
  • Next Minor Stable Release: patchset that needs to be verified in nightly before backport
  • Eventually: patchset that needs to be verified in alpha before backport
  • No Backport (preferred): patchset for the next major stable

(Optional) Justification

  • Emergency security update: patchset fixes CVEs, 0-days, etc
  • Censorship event: patchset enables censorship circumvention
  • Critical bug-fix: patchset fixes a bug in core-functionality
  • Consistency: patchset which would make development easier if it were in both the alpha and release branches; developer tools, build system changes, etc
  • Sponsor required: patchset required for sponsor
  • Localization: typos and other localization changes that should be also in the release branch
  • Other: please explain

Merging

  • Merge to tor-browser - !fixups to tor-browser-specific commits, new features, security backports
  • Merge to base-browser - !fixups to base-browser-specific commits, new features to be shared with mullvad-browser, and security backports
    • NOTE: if your changeset includes patches to both base-browser and tor-browser please clearly label in the change description which commits should be cherry-picked to base-browser after merging

Issue Tracking

Review

Request Reviewer

  • Request review from an applications developer depending on modified system:
    • @henry because they know these parts of code very well at this point and always give lot of very useful information
    • @dan/@clairehurst for the smallish Android part

Change Description

This started with removing localization from backend, and improving the general error reporting... eventually ended up adding also some other changes (reset the progress to 0 when going back to configuring, face more specific errors to the users in case of a tor startup error).

I don't know if this MR's approach is the best to solve these problems. Having localization in the backend is very ugly, but maybe should we create a separate middle-level module to pass translated strings to Android, to avoid repeating some strings? Notice that at the moment GV doesn't include any translation.

It should be very useful for Android to get the error codes. I thought of implementing them as an enum, but I didn't know how to create a conversion. After this change firefox-android breaks, but I will open a (draft) MR also for that, assuming we keep the same prototype.

I think this might help the Android work in the immediate terms... but I won't be here next week. So, if needed you can close this MR and someone else can take my branch, change it and merge the changed one instead.

Otherwise, if it's okay, I'll be back for changes on Monday 8.

This could probably be useful to aboutTorConnect.js itself, but I didn't want to make the MR too big (and we might end up doing a lot of changes there anyway in the future).

How Tested

  • Checked I didn't get strange errors on the bootstrap (also checked the censorship circumvention)
  • Checked the localization of the errors work as expected by trying to localize errors from the console
  • Tried to rename the tor binary, to trigger a startup error that says it cannot find it
  • I don't know how to create errors with a phase/reason from the tor side. I tried to rename the PTs, but that ended in tor crashing instead of reporting the error
Edited by Pier Angelo Vendrame

Merge request reports