BenVim / rapidjson

Automatically exported from code.google.com/p/rapidjson
MIT License
1 stars 0 forks source link

Better error handling. #79

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There are a number of conceptual problems with RAPIDJSON_PARSE_ERROR macro used 
for reporting errors while parsing JSON file.
1. It uses setjmp()/longjmp() mechanism for returning after error, it can be 
problematic on some platforms or in environment with exceptions (no stack 
unwinding, no RAII).
2. setjmp() part of code is doesn't uses macro, so it cannot be flexibly 
tuned/replaced.
3. You're reporting plain-text strings as error messages, it's not convenient 
in libraries (hard to guess for code what's happened, hard to localize). Better 
way is to use error codes instead of texts.
4. Also literal strings can be sub-optimal in pure-header templated classes 
because they tend to spawn a lot of copies in object modules that are not 
merged on linking if template is used in various places and/or various template 
args. This depends on compiler/linker power and options.

So, I propose:
1. Create an interface/templated concept class that handles reader errors. 
Create enum with a value for each possible error.
2. Pass it by reference/pointer to the reader in reader constructor, keep it in 
a member.
3. When error occurs invoke handler, stop processing and unwind reader class 
stack (simple "return false" and result checking will do the work without using 
exceptions).
4. Remove literal error strings completely.

Original issue reported on code.google.com by Anton.Br...@gmail.com on 14 Jun 2013 at 9:09

GoogleCodeExporter commented 8 years ago
I'm working on implementation of this proposal for my project, will post a 
patch for this later.

Original comment by Anton.Br...@gmail.com on 14 Jun 2013 at 9:11

GoogleCodeExporter commented 8 years ago
I agree most of your view. The error handling should be improved.

Thank you for your effort. 

Original comment by milo...@gmail.com on 18 Jun 2013 at 1:36

GoogleCodeExporter commented 8 years ago

Original comment by milo...@gmail.com on 18 Jun 2013 at 1:36

GoogleCodeExporter commented 8 years ago
Issue 16 has been merged into this issue.

Original comment by milo...@gmail.com on 24 Jun 2014 at 2:05

GoogleCodeExporter commented 8 years ago
https://github.com/miloyip/rapidjson/pull/27

Original comment by milo...@gmail.com on 30 Jun 2014 at 5:18