New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added logs and handle CompletableFutures exception #1600
Conversation
app/src/common/shared/org/mozilla/vrbrowser/search/suggestions/SuggestionsProvider.java
Show resolved
Hide resolved
@@ -90,6 +93,11 @@ public void setComparator(Comparator comparator) { | |||
items.sort(mComparator); | |||
} | |||
future.complete(items); | |||
|
|||
}).exceptionally(th -> { | |||
Log.e(LOGTAG, "Error getting bookmarks suggestions: " + th.getLocalizedMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the exceptions do not contain PII? I'm paranoid after all the work it took to clean up FxR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being part of AC I guess they don't, maybe @pocmo can answer that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would hope not but since these API deal with bookmarks and history I could conceive of something get leaked out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this calls into Kotlin and Rust code of AS libraries we can't really guarantee that at the AC level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keianhzo we should probably make the Logs debug then so they get stripped out for release?
3cb3077
to
c1e5d2e
Compare
@bluemarvin updated |
Fixes #1570 Better exception handling for Places related content in suggestions. There seems to be some problem with Places in x86_64 https://mozilla.slack.com/archives/CA4JKKLSG/p1565878585191200 already informed the AC team. In the meantime this helps to show the available content in the awesome bar instead of showing nothing.