Skip to content

Commit

Permalink
Downgrades certain NSAssert uses to a conditional log
Browse files Browse the repository at this point in the history
Summary:
We had a few buggy uses of NSAssert in the codebase, where what we really
wanted was a conditional log, in order to send the signal to a developer that
something may be amiss; but for which throwing an exception would be the
wrong remedy.

Test Plan: introduced unit test to test new logging macro, ran tests

Reviewers: clang, mmarucheck, gregschechte

Reviewed By: clang

CC: msdkexp@, platform-diffs@lists

Differential Revision: https://phabricator.fb.com/D546960

Task ID: 1368595
  • Loading branch information
Jason Clark committed Aug 13, 2012
1 parent 77984b8 commit 31bf78e
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/FBGraphObjectTableDataSource.m
Expand Up @@ -71,8 +71,8 @@ - (id)init

- (void)dealloc
{
NSAssert(![_pendingURLConnections count],
@"FBGraphObjectTableDataSource pending connection did not retain self");
FBConditionalLog(![_pendingURLConnections count],
@"FBGraphObjectTableDataSource pending connection did not retain self");

[_data release];
[_defaultPicture release];
Expand Down
4 changes: 2 additions & 2 deletions src/FBLogger.m
Expand Up @@ -185,8 +185,8 @@ + (void)registerCurrentTime:(NSString *)loggingBehavior
g_startTimesWithTags = [[NSMutableDictionary alloc] init];
}

NSAssert(g_startTimesWithTags.count < 1000,
@"Unexpectedly large number of outstanding perf logging start times, something is likely wrong.");
FBConditionalLog(g_startTimesWithTags.count < 1000,
@"Unexpectedly large number of outstanding perf logging start times, something is likely wrong.");

unsigned long currTime = [FBUtility currentTimeInMilliseconds];

Expand Down
3 changes: 2 additions & 1 deletion src/FBProfilePictureView.m
Expand Up @@ -17,6 +17,7 @@
#import "FBProfilePictureView.h"
#import "FBURLConnection.h"
#import "FBRequest.h"
#import "FBUtility.h"

@interface FBProfilePictureView()

Expand Down Expand Up @@ -155,7 +156,7 @@ - (void)refreshImage:(BOOL)forceRefresh {

FBURLConnectionHandler handler =
^(FBURLConnection *connection, NSError *error, NSURLResponse *response, NSData *data) {
NSAssert(self.connection == connection, @"Inconsistent connection state");
FBConditionalLog(self.connection == connection, @"Inconsistent connection state");

self.connection = nil;
if (!error) {
Expand Down
2 changes: 1 addition & 1 deletion src/FBSession.m
Expand Up @@ -394,7 +394,7 @@ - (BOOL)handleOpenURL:(NSURL *)url {
return [self handleOpenURLReauthorize:params
accessToken:accessToken];
default:
NSAssert(NO, @"handleOpenURL should not be called once a session has closed");
FBConditionalLog(NO, @"handleOpenURL should not be called once a session has closed");
return NO;
}
}
Expand Down
10 changes: 9 additions & 1 deletion src/FBUtility.h
Expand Up @@ -39,7 +39,15 @@
inBundle:(NSBundle *)bundle;

@end


#define FBConditionalLog(condition, desc, ...) \
do { \
if (!(condition)) { \
NSString *msg = [NSString stringWithFormat:(desc), ##__VA_ARGS__]; \
NSLog(@"FBConditionalLog: %@", msg); \
} \
} while(NO)

#define FB_BASE_URL @"facebook.com"


Expand Down
5 changes: 5 additions & 0 deletions src/tests/FBSessionTests.m
Expand Up @@ -20,6 +20,7 @@
#import "FBGraphUser.h"
#import "FBTestBlocker.h"
#import "FBTests.h"
#import "FBUtility.h"

#if defined(FACEBOOKSDK_SKIP_SESSION_TESTS)

Expand All @@ -32,6 +33,10 @@ @implementation FBSessionTests
// All code under test must be linked into the Unit Test bundle
- (void)testSessionBasic
{
FBConditionalLog(NO, @"Testing conditional %@", @"log");
FBConditionalLog(NO, @"Testing conditional log");
FBConditionalLog(YES, nil, @"Testing conditional log");

// create valid
FBTestBlocker *blocker = [[[FBTestBlocker alloc] init] autorelease];

Expand Down

0 comments on commit 31bf78e

Please sign in to comment.