Closed Bug 1685389 Opened 3 years ago Closed 3 years ago

Use callback.resolveTo instead of result.accept(callback.sendSuccess)

Categories

(GeckoView :: General, task, P3)

Unspecified
All

Tracking

(firefox86 fixed)

RESOLVED FIXED
86 Branch
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.

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

Assignee: nobody → agi
Status: NEW → ASSIGNED
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3cdc8e867f7d
Use callback.resolveTo instead of result.accept. r=esawin
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: