search

gomfunkel / node-exif

A node.js library to extract Exif metadata from images.
MIT License
581 stars 104 forks source link

extractMakernotes triggers error…should this be a warning? #21

Open fjhub opened 11 years ago

fjhub commented 11 years ago

Using version 0.3.3

I've come across a case in an image where there are maker notes and it is not found in the list of supported maker notes by this module. In looking at the outstanding issues, I noticed that issue #16 dealt with an exception handling issue. I manually made these changes to my version but did not get the desired result using the said image.

Even though there are maker notes in my image and they are not recognized, I think it would be a better solution to return the exifData less the maker notes rather than return an empty/null exifData object. Putting the call to 'this.extractmakernotes()' in a try/catch block solves this as the error can be caught which then allows the code to continue and return the exifData object.

Thus the maker notes error is treated more as a warning instead of a fatal error.

Any thoughts?

nocekt commented 11 years ago

I have also came across this issue as Markernotes for Canon are not supported. Yet, I believe it's not intended to throw an error in this case. Related code:

    } else  {
            // Makernotes are available but the format is not recognized so
            // an error message is pushed instead, this ain't the best 
            // solution but should do for now
            this.exifData.makernote['error'] = 'Unable to extract Makernote information as it is in an      unsupported or unrecognized format.';
    }
    this.exifData.makernote = this.extractMakernotes(data, self.makernoteOffset, tiffOffset);   
}
return this.exifData;

Else option is for unsupported Markernotes and in this case nothing is assigned to this.extractMakernotes. Thus error is thrown.

jayfresh commented 10 years ago

I noticed this is fixed in the current version in master, but npm still has 0.3.3.

stennie commented 10 years ago

Dupe of #19. Indeed appears to be fixed in 0.3.4+.

gomfunkel commented 10 years ago

Version 0.3.6 was published to npm a couple of minutes ago. Can you please check if the latest version solves your problem and leave a short feedback here? Thanks!