search

jeffcoughlin / farcrysolrpro

FarCry Solr Pro plugin Supports: Solr 3.5, FarCry 7.0+, 6.2+, 6.1.4+, 6.0.19+
http://jeffcoughlin.github.com/farcrysolrpro
Other
10 stars 8 forks source link

Better error handling around Tika errors #56

Closed smebberson closed 11 years ago

smebberson commented 11 years ago

At present, if Tika encounters and error, farcrySolrPro throws and error and stops indexing the remaining files.

I've updated the code so that it catches the error and reports it in the log, and keeps moving on with the indexing.

What do you think?

seancoyne commented 11 years ago

Good addition, but I think it would be better in the parseFile() function. If an error happens, log it and return a string. This way it keeps it out of the main method.

smebberson commented 11 years ago

So, should it return an empty string on fail otherwise it will return the content?

I don't know, that just doesn't seem as good as it could be. Then you'd have to check for string length on the result returned from parseFile. Maybe it would be better to go FarCry style and return a struct with a 'bSuccess' key, and return the result (if Tika parsing worked) in a 'content' key.

I'd be happy to code it up based on your thoughts...

seancoyne commented 11 years ago

I see what you're saying, but you wouldn't have to check for the length. The index would keep working w/ an empty string as the value for the contents.

smebberson commented 11 years ago

Oh yeah. Makes sense. I'll update the pull request with some new code.

seancoyne commented 11 years ago

I find it useful to also log the error message so change your log text to something like "tika failed to parse #filepath#, the error was: #e.message#" (assuming you keep the "e" error variable in your try/catch. Just more useful to see what the actual error thrown was.

smebberson commented 11 years ago

Hey Sean, I've updated the pull request with error handling moved into the parseFile function itself. Also took note of your wish for the error message and have formatted it as such.