Use callback.resolveTo instead of result.accept(callback.sendSuccess)
Categories
(GeckoView :: General, task, P3)
Tracking
(firefox86 fixed)
Tracking | Status | |
---|---|---|
firefox86 | --- | fixed |
People
(Reporter: agi, Assigned: agi)
Details
Attachments
(1 file)
We use the result.accept(... callback.sendSuccess(value))
pattern a lot in GV, but it's a little problematic because it doesn't handle the error case unless the developer explicitly adds a line to do that.
Instead we can use callback.resolveTo
which automatically rejects the callback when the result is rejected.
Patch incoming.
Assignee | ||
Comment 1•3 years ago
|
||
One thing I've noticed before going on break last year is that a lot of the
times we don't close out promises when an error occurs. Our code normally does
something like:
delegatePromise.then(response -> {
// manipulate response
callback.resolve(manipulatedResponse);
})
where delegatePromise
is the GeckoResult
from a delegate. The problem here
is that if the delegatePromise
is rejected, the above never rejects the
callback.
A month ago we introduced callback.resolveTo(result)
which obviates this
problem a little bit (it will rejects the callback if the result is rejected)
but it's still a little hard to use because you need to then the result to the
right value.
result.map
sort of closes this gap a little bit and now you can write
something like this:
callback.resolveTo(delegatePromise.map(value -> {
// manipulate value to match the response
return manipulatedValue;
});
Which is the recommended way to resolve callbacks in GV
Another problem with callback.reject is that we pretty much only support strings but it's not made really clear (some existing code in GV returns an exception there, which doesn't work). resolveTo will take care of that by using the message of the exception passed in
Updated•3 years ago
|
Pushed by asferro@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3cdc8e867f7d Use callback.resolveTo instead of result.accept. r=esawin
Comment 3•3 years ago
|
||
bugherder |
Description
•