Skip to content
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

Watchdog timer for products request. Observer protocol extension & some minor fixes. #60

Closed
wants to merge 2 commits into from

Conversation

IvanRublev
Copy link

About watchdog timer feature.

Watchdog will throw a timeout error in case of network lag on products request. It guarantees fixed time response to customer after his tap on Purchase button for in-app.

I didn't include example in commit but my use case is below.
In my apps after user taps "Purchase" button I show modal activity indicator view then make products request via RMStore. If I've got a products response before watchdog timer fires then there is a big probability that Apple servers will be reachable while payment transaction also. If network is bad I get timeout error from watchdog timer. Then I can choose to dismiss modal activity indicator view and place payment (with network activity indicator as in your example) or show "Try to Retry" message for customer in case of timeout error accordingly.

P.S. Thanks for great library. Keep rocking!

Ivan Rublev added 2 commits March 12, 2014 15:03
…able timeout. Feature is off by default.

Several setObject:forKey: changed to setValue:forKey:. Parameters checks against nil are added to prevent crashes.
…requests and transactions.

Useful to change UI on start and on finish of requests/transactions in commonplace.
@hpique
Copy link
Member

hpique commented Mar 14, 2014

Thanks for contributing code @IvanRublev! This appears to be a big pull request; I'll give it a good look as soon as possible.

At first glance, I don't think a timer feature should be included inside RMStore, as different apps might have different needs.

Wouldn't it be enough to check reachability before calling RMStore, and set a timer to check if DidFail/DidFinish notifications finished before a certain interval?

@IvanRublev
Copy link
Author

By default watchdog timer feature is disabled, and all processing is done as usual. Setting .useRequestProductsWatchdogTimer property to YES enables watchdog timer for further products requests.

When watchdog timer fires it cancels the request that was made via [self.request cancel] message, and request object is available only inside the class.

@hpique
Copy link
Member

hpique commented Mar 14, 2014

When watchdog timer fires it cancels the request that was made via [self.request cancel] message, and request object is available only inside the class.

So maybe what's really missing is a cancel request method in RMStore?

@IvanRublev
Copy link
Author

I think that canceling of request is used in very rare cases.
The point is that my feature just adds one more type of error to existent execution path without additional boilerplate. This is closing the lack of such error from Store Kit.
And all I want is just to have you to look at it closer, maybe you will like it :-)

@hpique
Copy link
Member

hpique commented Mar 14, 2014

I certainly will. :) I just want to understand the use case before looking at it.

The problem that you want to solve is that the payment timeout error that Store Kit provides takes too long, and you'd rather notify the user sooner. Correct?

From what you're telling me your solution assumes two things:

  1. That payment timeouts are the result of bottlenecks.
  2. That if a product request takes too long a payment request will too.

Backed by these two assumptions, you're using a cancellable product request as proxy for the payment request (which is not cancellable). If the product request takes too long, you cancel it and notify the user accordingly. And you couldn't do this outside of RMStore because the product request is not exposed.

Did I understand it correctly?

@IvanRublev
Copy link
Author

The problem that you want to solve is that the payment timeout error that Store Kit provides takes too long, and you'd rather notify the user sooner. Correct?

Exactly.

From what you're telling me your solution assumes two things:

  1. That payment timeouts are the result of bottlenecks.
  2. That if a product request takes too long a payment request will too.

Backed by these two assumptions, you're using a cancellable product request as proxy for the payment request (which is not cancellable). If the product request takes too long, you cancel it and notify the user accordingly. And you couldn't do this outside of RMStore because the product request is not exposed.

Did I understand it correctly?

Yes, you understand the solution correctly.

And another my assumption was that
3) Low latency and fast connection to any Internet site, for example apple.com, doesn't guarantee fast response from iTunes servers.
That what exactly I had a couple of month ago, and what was the reason for such solution.

@@ -192,9 +217,14 @@ - (void)addPayment:(NSString*)productIdentifier
RMAddPaymentParameters *parameters = [[RMAddPaymentParameters alloc] init];
parameters.successBlock = successBlock;
parameters.failureBlock = failureBlock;
[_addPaymentParameters setObject:parameters forKey:productIdentifier];
[_addPaymentParameters setValue:parameters forKey:productIdentifier];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change to KVC's setValue?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent crash if parameters == nil. (Extra safety)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't be using KVC for that. And here in particular, it's not possible for parameters to be nil.

@hpique
Copy link
Member

hpique commented Mar 16, 2014

Hi @IvanRublev.

This feature is for a very specific use case, and as such is not aligned with the spirit of RMStore, which is providing a higher level of abstraction over Store Kit APIs and very little else. Also, I'm not comfortable with adding code based on empirical assumptions. What works today might not work tomorrow.

In any case, thanks for contributing code and an interesting idea to improve user experience when there are connectivity issues. I left some comments on the code about the minor fixes.

@hpique hpique closed this Mar 16, 2014
@IvanRublev
Copy link
Author

Of course, It's up to you. :-)
I've commented back about minor fixes, hope my comments may be useful also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants