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

Over-released NSError in renderObject:fromContentsOfFile:error: #6

Closed
pix0r opened this issue Jul 26, 2011 · 12 comments
Closed

Over-released NSError in renderObject:fromContentsOfFile:error: #6

pix0r opened this issue Jul 26, 2011 · 12 comments
Labels

Comments

@pix0r
Copy link
Contributor

pix0r commented Jul 26, 2011

I'm having an issue getting the output NSError from GRMustacheTemplate +renderObject:fromContentsOfFile:error:. When an error is encountered, the NSError** I passed in is over-released.

The offending code, I believe, is the explicit autorelease pool set up in https://github.com/groue/GRMustache/blob/master/Classes/GRMustacheRendering.m#L201

When an error is encountered, GRMustacheTemplateParser calls -finishWithError: which sets its error ivar (retained).

The problem is that the template/parser is released when the NSAutoreleasePool is drained, thus the error is released. When execution returns back to my code, the error has already been released and if I'll get a violation if I attempt to access it.

@groue
Copy link
Owner

groue commented Jul 27, 2011

This issue has been closed thanks to your pull request. Thank you very much!

@groue groue closed this as completed Jul 27, 2011
@pawan1011
Copy link

Hi,
We are facing a issue with this method in Mustache. [GRMustacheTemplate renderObject: fromString: error: ];
So whenever a error is encountered am printing it NSLog(@"error is %@",[err userInfo]);
But a get a BAD_ACCESS and the app crashes here. Am using the latest version of Mustache.
What exactly is the issue here?

@groue
Copy link
Owner

groue commented Aug 22, 2012

Oops. Investigating.

@groue
Copy link
Owner

groue commented Aug 22, 2012

@pawan1011 I can not reproduce your issue.

Here is some code that works fine, for instance:

#import <Foundation/Foundation.h>
#import "GRMustache.h"

int main(int argc, const char * argv[])
{
    @autoreleasepool {
        NSError *error;
        id data = @{};
        NSString *rendering = [GRMustacheTemplate renderObject:data fromString:@"{{fail" error:&error];
        if (!rendering) {
            // works fine
            NSLog(@"error is %@",[error userInfo]);
        }
    }
    return 0;
}

Please attach some minimal code that exhibits this crash, so that we figure out where is your problem.

@pawan1011
Copy link

NSString *tempStr= [[NSString alloc ]initWithString:@"Replacable String"];
NSMutableDictionary *data = [NSMutableDictionary dictionaryWithObject:tempStr forKey:@"context"];
NSString *rendering = [GRMustacheTemplate renderObject:data
fromString:@"Test {{contextPath}} "
error:&err];
if (!rendering) {
NSLog(@"error is %@",[err userInfo]);
}

I assumed GRMustache will throw a error for the above code since the Templates do not match. But it did not..!!
My question here is when will I get a error message from Mustache? Can you clarify.

@groue
Copy link
Owner

groue commented Aug 22, 2012

Sure I can.

Errors are documented at https://github.com/groue/GRMustache/blob/master/Guides/templates.md :

GRMustache methods may return errors whose domain is GRMustacheErrorDomain, and error codes interpreted with the GRMustacheErrorCode enumeration:

extern NSString* const GRMustacheErrorDomain;

typedef enum {
    GRMustacheErrorCodeParseError,
    GRMustacheErrorCodeTemplateNotFound,
} GRMustacheErrorCode;

That means that the only errors you get are parse errors (for unparsable templates), and template not found errors (when loading a template file that does not exist, for instance).

Your template, Test {{contextPath}}, is perfectly valid Mustache, and does not load any partial template that could be missing, so you won't get any error.

The mismatch you are experiencing is between the contextPath key in the template, and the context key in your data.

GRMustache has absolutely no opinion on keys that should be provided by data. If data provides contextPath, GRMustache will render the associated value. If data does not provide contextPath, nothing will be rendered. And actually, your rendering variable contains a string: Test. The {{contextPath}} was not tied to any value, and has been replaced by an empty string.

This is the way GRMustache works: missing keys are not errors. They are just not rendered at all. You may find this behavior annoying. Actually, it is quite useful: check https://github.com/groue/GRMustache/blob/master/Guides/runtime/context_stack.md for the big picture.

But let's get back to your problem: since your sample template is quite tiny, it's not very difficult to "fix". However, real-life templates can get huge. It may become tedious to spot this kind of errors. If this is your case, I encourage you reading https://github.com/groue/GRMustache/blob/master/Guides/delegate.md : you may find it useful.

Last point:

GRMustache returns errors in the conventional Objective-C way ( https://developer.apple.com/library/ios/#documentation/Cocoa/Conceptual/ErrorHandlingCocoa/CreateCustomizeNSError/CreateCustomizeNSError.html). Particularly (emphasis mine):

Important: Success or failure is indicated by the return value of the method. Although Cocoa methods that indirectly return error objects in the Cocoa error domain are guaranteed to return such objects if the method indicates failure by directly returning nil or NO, you should always check that the return value is nil or NO before attempting to do anything with the NSError object.

In your particular case, since renderObject:fromString:error did return a rendering, checking the error was a mistake.

I hope things are clearer, now :-)

@pawan1011
Copy link

Yes.. Now I am able to get 'parse errors ' when the template is of type {{contextPath.
But the second error type template not found errors (when loading a template file that does not exist, for instance) that you mentioned above is not working.
I have given wrong template file path in my code. But still no error message.
Can you clarify on the second error condition.

@groue
Copy link
Owner

groue commented Aug 23, 2012

Please attach some code again.

@pawan1011
Copy link

NSString *tempStr= [[NSString alloc ]initWithString:@"Rendering String"];
NSError *err ;
NSString *templateString = [NSString stringWithContentsOfFile:[NSString stringWithFormat:@"%@/Desktop/mustache.xml",NSHomeDirectory()] encoding:NSUTF8StringEncoding error:NULL];
NSMutableDictionary *data = [NSMutableDictionary dictionaryWithObject:tempStr forKey:@"contextPath"];
NSString *rendering = [GRMustacheTemplate renderObject:data
                                            fromString:templateString                
                                                 error:&err];

if (!rendering) {
    NSLog(@"error is %@",[err localizedDescription]);
}

Here, I do not have any file by name mustache.xml in that file path specified. But still, Mustache is not throwing any error as expected.

@groue
Copy link
Owner

groue commented Aug 23, 2012

Look a little closely, with focus and dedication, at the way you invoke GRMustache: you use the renderObject:fromString:error method. This method does absolutely not talk about files. It talks about strings. Do you know the -[NSString originalFile] method? No, you don't, because it doesn't exist. So GRMustache can't tell you that the file mustache.xml does not exist. And it doesn't, as you can see.

Let's go further: since the file does not exist, templateString is nil. Did you notice? No, because you haven't tested the result of stringWithContentsOfFile:encoding:error:. That's unfortunate, because now you are invoking [GRMustacheTemplate renderObject:data fromString:nil error:&err] (nil string) without even knowing it. The program fails again, because your control over it is lacking.

Your two options now:

  1. either test the result of stringWithContentsOfFile:encoding:error: : you'll get an early error, and you'll know that it's useless to invoke GRMustache.
  2. use the [GRMustacheTemplate renderObject:fromContentsOfFile:error:] method, which takes a path, not a string, and that will return you an error when the file does not exist.

@pawan1011
Copy link

Yes groue..I did test and notice that since file does not exist in the path specified ,templateString will be nil.
But what I misunderstood was [GRMustacheTemplate renderObject: fromString: error:] method will check if the file exists,
if not throw an error, since we are reading its contents and passing the NSString as a parameter to that method.

Well, Thanks for clarifying that only [GRMustacheTemplate renderObject:fromContentsOfFile:error:] method will throw an error if the file does not exist in the path specified.

@groue
Copy link
Owner

groue commented Aug 23, 2012

You're welcome. Happy Mustache!

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

No branches or pull requests

3 participants