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
Subscriptions: fixing fallback after local platform failure #18363
Subscriptions: fixing fallback after local platform failure #18363
Conversation
prateekbh
commented
Sep 25, 2018
- Shifts resolution of fallback entitlement right after local platform fails
Can you add a test for the case where non-local fails to make sure it doesn't use fallback. |
@@ -468,18 +468,17 @@ export class PlatformStore { | |||
* @param {string} serviceId | |||
*/ | |||
reportPlatformFailure(serviceId) { | |||
if (this.failedPlatforms_.indexOf(serviceId) == -1) { | |||
if (serviceId === this.getLocalPlatform().getServiceId() | |||
&& this.fallbackEntitlement_) { |
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.
What would happen if the fallbackEntitlement
is successful, but other services have not finished yet? Will we execute the "local" renderer and send "local" pingback even though some other services might return with an actual positive response? Or does "renderer" and "pingback" wait for other services anyway?
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.
Well in theory its just filling local with a proxied Entitlement and doesn't change anything with selectPlatform
flow. But will check and see if we have a test for that
@dvoytenko @jpettitt it indeed unblocks in favor of local always, not merging this right now. |
…ct#18363) * fixing fallback on local platform only * adding negation test * adding more verbose test * fixing lint
…ct#18363) * fixing fallback on local platform only * adding negation test * adding more verbose test * fixing lint