aomader / ompromises Goto Github PK

Hi,
I have been integrating OMPromises on our app and our code analysis tool picked up a potential problem on OMDeferred.m in the following method:

- (void)progress:(float)progress 

The initialisation of

    NSArray *progressHandlers = nil;

    @synchronized (self) {
    // ... Initialising progressHandlers with pending handlers.
    }

    @synchronized (progressHandlers) {
        for (void (^progressHandler)(float) in progressHandlers) {
            progressHandler(progress);
        }
    }

Static analysis complains about progressHandlers potentially being nil in which cast the lock will never happen and no synchronisation will happen.

Based on my understanding I think that scenario reported is not real although, couldn't we allocate an empty array to prevent this analysis issue? I have tested it and it seems to pass the tests.

I've been using the chain: method quite a bit, and I was wondering if you'd thought about the ability to insert rescue and failure blocks into the chain in addition to then blocks?

I have have the following setup:

@property (nonatomic, strong) OMDeferred *didLoad;

{
    ..
    [self.didLoad fulfil:self];
    ..
}

The problem is now that this causes a retain cycle that I have to break by nil'ing didLoad.
Does the OMDeferred really need to hold a strong reference?

Once Travis CI supports XCode 5, use it to continuously test the product.
See the related issue: travis-ci/travis-ci#1422

Just as an example - say I want to wait with a OMPromise chain for the first time the view did appear - which by definition this can happen multiple times. Which also is why the following code will fail.

@property(nonatomic) OMDeferred *didAppear;

- (id) init...
{
  self.didAppear = [OMDeferred deferred];
}

- (void) viewDidAppear
{
  [super viewDidAppear];
  [self.didAppear fulfil:self];
}

I guess I could work around this by checking the state of the OMPromise but I was hoping for a more elegant way of expressing this.

I'm running my application with Xcode 5 and iOS 6.0 simulator. XCTest is not supported in iOS 6.0. App will crash with this:

dyld: Library not loaded: /Developer/Library/Frameworks/XCTest.framework/XCTest
Referenced from: /Users/visavarjus/Library/Application Support/iPhone Simulator/6.0/Applications/B6B1365F-308E-49A7-B545-B8409F329643/W-ZUP.app/W-ZUP
Reason: image not found

Currently, "Tests" subspec gets installed, even though we don't need it for app targets. Maybe OMPromises.podspec should define a "core" subspec and use it as default_subspec.

When returning an already fulfilled promise from a then block an assertion gets raised causing the app to crash.

Code to test:

[[somePromise then: ^( id result ) {
   return [OMPromise promiseWithResult: @42];
}] then: ^id (id result) {
   NSLog( @"The answer: %@", result );
   return result;
}];

I know this is a rather contrived example, but I am also running into this issue in my real app.

Console output:

2014-03-16 12:47:36.369 TestApp[85851:60b] *** Assertion failure in -[OMDeferred progress:], .../Pods/OMPromises/OMPromises/OMDeferred.m:84
2014-03-16 12:51:17.389 TestApp[85851:60b] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Can only progress while being Unfulfilled'
*** First throw call stack:
(
    0   CoreFoundation                      0x01cbf1e4 __exceptionPreprocess + 180
    1   libobjc.A.dylib                     0x01a3e8e5 objc_exception_throw + 44
    2   CoreFoundation                      0x01cbf048 +[NSException raise:format:arguments:] + 136
    3   Foundation                          0x003b54de -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 116
    4   TestApp                             0x0002693a -[OMDeferred progress:] + 378
    5   TestApp                             0x0002820d __21-[OMPromise then:on:]_block_invoke_2 + 125
    6   TestApp                             0x00029ce6 -[OMPromise progressed:on:] + 326
    7   TestApp                             0x00029b55 -[OMPromise progressed:] + 101
    8   TestApp                             0x00028131 __21-[OMPromise then:on:]_block_invoke47 + 241
    9   TestApp                             0x0002633b -[OMDeferred fulfil:] + 571
    10  TestApp                             0x0002c944 __17+[OMPromise all:]_block_invoke186 + 260
    11  TestApp                             0x0002633b -[OMDeferred fulfil:] + 571

I have concurrent requests to an API. The calls to the API need to be authenticated. Each authentication request creates a new token. Now the problem is that I don't want the API to create a token per concurrent request. Only the first authentication call should lead to a request and a single token.

My callback base implementation uses a dispatch group.
Now I am looking for the right pattern when using promises.
Simple chaining does not seem to be enough here.

Any suggestions?

I would rename fulfilled: to succeeded:. Reads much better. Using fulfilled just because it's a promise feels like trying too hard.

When I run my app with Xcode's Thread sanitizer enabled, I get an issue reported where read access to the 'state' property is not properly guarded (see attached screenshot). This could be fixed by either declaring the 'state' property as atomic, or by putting the read access within the @synchronized clause.

Any ideas what would be the best route to fix this?

It would be good to create a promise of a synchronous method with its return value being the result of the promise. Something like the below.

OMPromise *blockResult = [OMPromise run:^{
    // that takes verryy long
    return myAwesomeResult;
}];

OMPromise *methodResult = [OMPromise call:@select(foo:) on:self];

The actual call of the method/block should be performed in a background thread and the result/failure should be deployed in the thread where the promise has been created.

From Time to time I see assertion failure
https://github.com/b52/OMPromises/blob/master/Classes/Core/OMDeferred.m#L90
NSAssert(self.progress <= progress, @"Progress can only increase");

Even though the progress is equal.

Please have a look at this article.
http://stackoverflow.com/questions/10334688/how-dangerous-is-it-to-compare-floating-point-values

In the comments you can find explanation and solution for the issue.

My suggestion for a quick fix:
NSAssert(self.progress - progress < 0.001, @"Progress can only increase");

The key error_exception_%@ cannot be found, that causes the format to be nil and gives another exception overshadowing the original one.

https://github.com/b52/OMPromises/blob/master/Classes/Core/OMResources.m#L67

Frankly speaking I don't understand how though. Quoting from the docs

A localized version of the string designated by key in table tableName. If value is nil or an empty string, and a localized string is not found in the table, returns key. If key and value are both nil, returns the empty string.

Yet I see

(lldb) po key
error_exception_%@

(lldb) po format
 nil

Any idea? Maybe just add a safe guard?

Are there any plans to add tvOS as a supported platform?

Check what is needed in order to use OMPromises with OS X.

When you do
[OMPromise any:@[A, B, C]];
and e.g. B is fulfilled.
Wouldn't it be cool to cancel A and C.
It's not a big deal, but users may expect such behavior, special when there is a cancel method on promises.
Same is true for progress. If we have an any concatenation, we are done (100% progress) when any of the promises is fulfilled.

This code

- (OMPromise*) test
{
    return [OMPromise promiseWithResult:nil];
}

- (OMPromise *) test2
{
    return [[self test] then:^id(id arg) {
        return [OMPromise promiseWithResult:nil];
    }];
}

yields an exception "Can only progress while being Unfulfilled" and I don't see anything wrong with it.

Add general documentation to describe the overall working procedure of OMPromise and OMDeferred.

In iOS 8 the [NSLocale preferredLanguages] array may contain only one entry. For example de. This results in a crash in OMResources line 67 because OMLocalizedStrings() returns nil.

Create a podspec and distribute it.

Another pattern question:

The API has two methods doOne and doTwo. I need the final result of both calls hence:

OMPromise *all = [OMPromise all:@[
    [[API sharedInstance] doOne],
    [[API sharedInstance] doTwo]
]];

[[all fulfilled:^(id result) {
 ...
}] failed:^(NSError *error) {
 ....
}];

Now the problem is that doTwo may fail with a 401 status code. In fact the result is optional. With the above code snippet it's required and means the whole combined promise fails, too.

I know I could wrap the doTwo in another promise that catches the error and always fulfills.

OMDeferred *optional = [OMDeferred deferred];

[[[[API sharedInstance] doTwo] fulfilled:^(id result) {
    [optional fulfil:result];
}] failed:^(NSError *error) {
    [optional fulfil:nil];
}];

OMPromise *all = [OMPromise all:@[
    [[API sharedInstance] doOne],
    optional.promise
]];

[[all fulfilled:^(id result) {

    GTLog(@"%@", result);

}] failed:^(NSError *error) {

    GTErr(@"%@", error);

}];

But is there a more elegant way?

• (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error {

crashes if self.response is not initialised.

The combinators defined in Control.Monad might be potential candidates to port to OMPromises.
Check which one might be useful by finding appropriate use-cases and implement them, tests first of course.

I wrote following category for my self:
https://gist.github.com/mzaks/7c53b0df161f62eac535

Use case for this was: An API which returns failed promise if data could be fetched but could not be validated.

A small enhancement but maybe you want to add it to the library.

It would be nice to allow some sort of cancellation. Meaning that the user of the promise can indicate that he is no longer interested in the result of the promise. Therefore the underlying deferred could interrupt the ongoing operation, if possible.