-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
JsonArray.add(JsonObject) adds partial JsonObject when the JsonDocument is full #2081
Comments
Anticipating the possible response that may come, in my project's API, I don't have direct access to the JsonDocument, so I can't use the capacity() memoryUsage() methods to try to workaround this issue. |
Ok. I worked around it keeping a count of the JsonObject that were successfully added to the JsonArray and when it exits the loop (because the last addition failed due to memory), I compare the |
Hi @gonzabrusco, I guess I could add a special treatment for Also, this "transactional guarantee" would be very limited since you'd not necessarily return to the state before the operation. For example, if add failed in the following program: doc["config"]["wifi"].add(newAccessPointObject); If {"config":{"url":"api.example.com"}} After "reverting" the failing {"config":{"url":"api.example.com","wifi":[]}} It is probably okay, but definitely not the same as the original. Moreover, because ArduinoJson 6's allocator is monotonic, we cannot recover the memory consumed by the removed object. Best regards, |
Yes I understand what you are saying but I still think that what I propose would be the correct choice in this regard. If the add method fails, it should not leave "a partial copy" of a JSON. In the example you provided, it "fails" because you are doing two things at a time (creating an array and then adding an item). I think in that case the user should use JsonDocument::createNestedArray() and check the return value before adding items. Or just leave the array empty (which is probably fine because the other end is probably expecting the array, empty or not). |
I worked several hours on this issue but could not find an elegant solution that doesn't increase the code size significantly. |
Fair enough. I will stick to my workaround then. Thank you for taking the time to evaluate this issue. Feel free to what you want with this issue report. |
Describe the bug
Not sure if a bug or a design choice. But this is the workflow to reproduce this issue:
{"id":3,"test":"this is a test", "another_key":"random text"}
JsonArray.add()
returns false (no more memory in the JsonDocument to add another item in the array).When checking what was saved in the JsonArray, I find something like this:
It seems that when you call the method
add()
it returns false if there's no more memory to fully add the JsonObject, but it leaves a partial JSON in the array. I think this should not be the correct behavior. It should notify if there's not enough memory to store the JsonObject but leave the array untouched. Adding a partial JSON probably is a bad idea in most situations (API breaker).The text was updated successfully, but these errors were encountered: