- Switched to using workspace dependencies, this will make future
updates much simpler
- Updated code to work with the new UniFFI crate structure
- Updated the error-support integration tests to work with the workspace
dependencies, following this advice: https://stackoverflow.com/a/71461114.
Went through all the main app-services components and replaced the
old-style `log::error!` calls with `error_support::report_error!`.
Removed the `reporting` feature of `error-support` and made it
always-enabled.
Renamed the `report_error` function so that it's name doesn't clash with
the `report_error` macro
I think this name communicates it's purpose better and is more
consistent with other components.
Renamed `LoginsError` to `Error` for the same reasons as the change from
`PlacesError` to `Error`
Changed `logins.udl` to use the fielded error style. Updated
`LoginsApiError` to use named fields to be compatibile with this.
Removed the `RequestFailed` variant, based on the note next to it.
- Renamed `PlacesInternalError` to `Error` because it seems more
idiomatic
- Renamed `PlacesError` to `PlacesApiError` to make it consistent with
the logins errors. I considered just calling this `ApiError` but I
thought that would be confusing for consumers since they won't always
have the `places::` prefix before the class name.
- Removed `JsonError`, which seems like it was unusped
- Merged `InternalError` into `UnexpectedPlacesException`
- Merged `UrlTooLong` into `UrlParseFailed` for `PlacesApiError`
- Merged `InvalidBookmarkUpdate`, `CannotUpdateRoot`, `InvalidParent`
into `InvalidBookmarkOperation` for `PlacesApiError`
Use the fielded style for `PlacesApiError` in places.udl. Updated
`PlacesApiError` to use named fields to be compatible with this.
On swift renamed `PlacesApiError`, which was defined on top of the
normal errors to `PlacesConnectionError`.
Replaced the error_support method with the new system that consists of:
- `PlacesInternalError`: used internally in the crate. This replaces
the old `ErrorKind` enum and is the error for `Result<>`.
- `PlacesError`: used for external errors that we return to UniFFI
consumers. This replaces the old `Error` enum and is the error for
`ApiResult<>`
- A `GetErrorHandling` impl to convert internal errors to external
errors and report to sentry. For now, I tried to make the sentry
error reporting more-or-less match the old error reporting, which
means reporting lots of internal errors. The plan is going to be to
check sentry and most likely turn some of those into logs rather
than errors.
Reworked all top-level API methods to return `PlacesApiResults` and use
the `handle_error!` macro to do the error conversion/reporting.
Use `thiserror` to handle converting errors from other crates into
`PlacesInternalError`.
sync-test was removed from the workspace some time ago, which meant it
was never built or checked, so it went quite stale, particularly with
respect to logins changes.
We not keep it in the workspace but as an "excluded" item, which means
you can run it from its directory. To prevent it going stale again, CI
runs clippy on it.
Also needed a few misc tweaks to some other crates.
If the new feature is enabled, tabs can sync - otherwise it can not.
The intention is that later there will be a "bridged-sync" feature,
and that in-practice, one or the other will be enabled - however, the
way features work, it must be possible that no features are enabled.
The new feature is not a default feature because I expect we will
want the default features to be what desktop consumes - ie, that
the default feature will be bridged-sync once that exists.
This isn't strictly required yet, but is being done now to make
future reviews of the bridged engine easier to reason about.
(Which further reinforces that the name of that crate should be something like
`sync15-types`)
This is preparatory work for a much larger patch that will get us further down
the path to #2841 and also fix#2712. It should not change any behaviour.
As part of this, the sync15_traits errors needs to be addressed. It's not
reasonable to just create another enum that we expect clients to add to
their error types because many of the errors overlap which makes handling
them very difficult (eg, if we want to catch a serde error, you don't want
to know which crate it originated in) - so sync15 duplicates the sync15_traits
errors and can convert between them - so consumers can continue to catch
sync15::Error and still get errors which originated in sync15-traits.
sync15-traits now has a `crypto` feature - sync15 enables that feature, but
crates which just use a `bridged_engine` do not. This means that when
vendoring into Desktop Firefox, this feature isn't enabled, so we don't
pull in any of the crypto libs there.
- Use `thiserror` only to define our errors rather than a weird
combination of `thiserror` and `error_support`
- There's now a few error enums:
- `LoginsStorageError` is for public API errors
- `LoginsError` is for internal errors
- `InvalidLoginsReason` is a subtype for `LoginsError::InvalidLogin`
- Extracted the error tracking code from `impl From<LoginsError>
for LoginsStorageError` and put it in its own methods.
- Made `LoginsError::InvalidKey` convert to
`LoginsStorageError::CryptoError`. I think this makes sense since
from the app's point of view an invalid key is the same thing as
data that can't be decrypted. Note: that `InvalidKey` used to mean
something different when we were using sqlcipher databases, but that's
not applicable anymore.
- Dropped the `throw!` macro. The error types are simple enough now
that we can just use a return statement.
- Added note about PII
- Renamed `LoginsError::label()` to `LoginsError::group_name()`
- Updated the rest of the code to use the new API
- Removed the backtrace functionality from `sync-pass`. I'm kind of
doubt it was working anyways.
Report breadcrumbs for:
- Syncing
- History DB operations that delete rows.
- The delete_everything transaction
Added code to report errors from `delete_everything_history`.
Added changelog entry
* Update rand and parking_lot.
I need this to update other Gecko dependencies.
* Update dependency_summary to account for various windows-rs crates.
I'm not sure why it wasn't failing to find the relevant licenses before,
it was likely a pre-existing issue.
* Update dependency summaries.
* Runs dependency summaries on a mac
Co-authored-by: Emilio Cobos Álvarez <emilio@crisal.io>
- Refactored the `SqlInterruptHandle` and `SqlInterruptScope` types to
work better with the new code.
- Types now store an `SqlInterruptHandle` and use that to create
`SqlInterruptScope` instances. This seems more natural than storing
an `Arc<AtomicUsize>` and using that to create both the scope and
handle.
- Creating an `SqlInterruptScope` can fail if we are in shutdown mode.
Updated the calling code to handle this error.
- We now use `Ordering::Relaxed`. This has less overhead compared to
`Ordering::SeqCst` and I'm pretty sure we don't need the extra
synchronization guarantees
- Moved the `Interrupted` and `Interuptee` types to their own modules.
Updated the shutdown module documentation.
- Changed the `PlacesApi` sync methods to to just send `NeverInterrupts`
to the `sync_multiple()`. I think this is fine given that we only use
these methods on iOS and we don't have actual interruption support
there yet.
- Removed the `PlacesApi::new_sync_conn_interrupt_handle()` method. If we want
to interrupt syncing, then we should just enter shutdown mode.
The `SqlInterruptHandle` and `SqlInterruptScope` types and especially
the shutdown code feels like belongs in `interrupt-support` better than
`sql-support`.
Removed the `implement_into_ffi_by_pointer!(SqlInterruptHandle)` call.
I don't think we need that now that everything is UniFFIed.
* Prepare Nimbus for using uniffi Custom types
* Update places to use renamed Custom types
* Increment uniffi version number
* Update Cargo.lock
* Fixup ktlint
* initial conversion of errors for places uniffication
* Converted history metadata functions for places uniffication
* Converted kotlin history metadata functions
* Converted iOS history metadata functions
* [Places uniffication] Fix places error issues (#4687)
* remove uniffi-specific wrapper and updated error test
* Uniffi VisitObservation and apply_observation (#4689)
* uniffied VisitObservation, HistoryVisitInfo and methods using those structs
* Uniffi top frecent site info (#4711)
* Uniffi top frecent site info
* Uniffis FrecencyThresholdOption
* Uniffi Places Sync (#4714)
* uniffi places sync and sync15 functions
* Uniffi Search result and match url (#4720)
* Uniffi query autocomplete
* uniffi places_accept_result
* uniffi places_match_url
* cargo fmt
* cargo clippy
* remove accept_result from ffi crate
* Adds comment on the uniffi bug with varaint shadowing type
* Uniffi Places Interrupt methods (#4726)
* uniffi places interrupt functions
* Consolidate types (#4736)
* Updates the visit observation to use URL
* Uses VisitTransition for HistoryVisitInfo
* updates uniffi to 0.16
* Uniffied bookmarks
* switch to use uint in the apis for adding bookmarks (#4747)
* Remove the manual places FFI entirely.
* folders and separators now have non-nullable parents
* Always insert via Insertable items, and clarify separation between node types.
* Add json_tree module with all json-based types and implementation.
* Fix swift warnings re non-null parentGuid
* bookmarks: Remove public_node and introduce a fetch module
* Update swift code to make breaking changes fixes cleaner (#4766)
* rebased and added changelog entry
Co-authored-by: lougeniac64 <lougeniaC64@users.noreply.github.com>
Co-authored-by: Tarik Eshaq <teshaq@mozilla.com>
Co-authored-by: Mark Hammond <mhammond@skippinet.com.au>
Added an enum that contains all possible sync engines and a method to
iterate over registered sync engines. Used that, plus the existing
`get_engine()` method to remove a lot of redundant code.
Use "passwords" to identify the logins engine in `wipe()`, `reset()` and
other methods (#4728)
- Replaced hand-written FFI code with UniFFI code
- Updated error handling to just use thiserror. There's no need for
error-support anymore.
- Added an enum for the SyncParams.engine. This seems cleaner than
having None mean sync all engines.
- Replaced `SyncResult.results` with `SyncResult.successful` and
`SyncSyncResult.failures`. Before this was handled in the Kotlin
wrapaper code.
- Made the megazord depend on tabs directly. Before it pulled in tabs
because of a transitive dependency from sync_manager_ffi.
Prior to this commit, the `build.gradle` for each individual
project contained copy-pasted logic for configuring Android
and protobuf. Some of them were even using slightly different
versions of the same dependencies.
With this commit, there are two new gradle scripts that encapsulate
this shared logic:
* ./build-scripts/component-common.gradle, for basic Android
and Kotlin setup.
* ./build-scripts/protobuf-common.gradle, for configuration specific
to the protobuf plugin.
Hopefully this will make the logic easier to maintain going forward.
Prior to this commit, each of our individual components had its
own copy of gradle logic for depending on the megazord, including
a couple of lengthy comments about fiddling around with JNA for
the Android Studio test runner.
I've moved all that logic into a helper function in `publish.gradle`
so that we can maintain it in a single location from here out.
There may be a cleaner or more idiomatic way to achieve this kind
of code-sharing in gradle, but I figure this *has* to be better
than our current copy-paste-based approach.
I've manually confirmed that a local `./gradlew test` completes
successfully, as well as testing via Android Studio.
lougeniac64
Ben Dean-Kawamura
Sammy Khamis
Mark Hammond
lougeniaC64
Tarik Eshaq
jhugman
Sammy Khamis
Ryan Kelly
Ryan Kelly