Adds JSONException for write value errors

[johnjaylward]

What problem does this code solve?
This wraps exceptions that may occur during a write operation on JSONObject or JSONArray. The message include the key or index for which the error occured. This provides easier tracing for errors in log messages and exception stacks.

example log before the change:

at org.json.JSONObject.write(line: 2234)
at org.json.JSONObject.write(line: 2249)
at org.json.JSONObject.write(line: 2234)
at org.json.JSONObject.write(line: 2249)
at org.json.JSONObject.write(line: 2234)
...

Example after the change:

at org.json.JSONObject.write(line: 2234)
at org.json.JSONObject.write(line: 2249): Unable to write JSONObject value for key: root
at org.json.JSONObject.write(line: 2234)
at org.json.JSONObject.write(line: 2249): Unable to write JSONObject value for key: childObjectKey
at org.json.JSONObject.write(line: 2234)
...

Risks
low. There would be some extra overhead on the stack traces, but really at this point in the code it resolves as a developer error. If these exceptions are being hit regularly in production code, the software has bigger problems than a little extra exception overhead.

In my case it was when I was trying to serialize a hibernate object outside of a hibernate session. In production systems, something like that should never happen. It happened to me because the error was thrown from an implementation of toString that was requesting a lazy-loaded object (which was unavailable because the object was detatched from a hibernate session)

Changes to the API?
No changes

Will this require a new release?
No, can be rolled into the next release

Should the documentation be updated?
No

Does it break the unit tests?
Yes. 1 test case failed because the error message is now wrapped. The test case has been updated and some test cases have been extended to increase code coverage.

see PR stleary/JSON-Java-unit-test#73

Was any code refactored in this commit?
Yes

Review status
ACCEPTED Starting 3 day comment window

[stleary]

Looks like a reasonable change. However, I think it breaks the unit tests, so some change may be needed to sync to the new behavior. Also, new tests should be added to cover the new code and confirm the new behavior.

org.json.junit.JSONStringTest > testJSONStringExceptionValue FAILED
java.lang.AssertionError at JSONStringTest.java:245

org.json.junit.JunitTestSuite > org.json.junit.JSONStringTest.testJSONStringExceptionValue FAILED
java.lang.AssertionError at JSONStringTest.java:245

480 tests completed, 2 failed

[johnjaylward]

you are correct. I somehow missed those failed tests. I'll add a PR to the test project

[johnjaylward]

Updated the unit tests and linked it in the PR description here.

[stleary]

Thanks for the unit tests for this feature and the additional test coverage. Looks good to go, starting 3 day window.