fix(rdb_load): Handle JSON loading failure when parsing fails #4801
Conversation
| if (json) { | ||
| pv_->SetJson(std::move(*json)); | ||
| } else { | ||
| LOG(ERROR) << "Invalid JSON string"; |
There was a problem hiding this comment.
Won't this trigger errors in productions ? Is that something that we want ? cc @adiholden
There was a problem hiding this comment.
I checked other methods and they log ERROR for similar issues
There was a problem hiding this comment.
we usually print error log for internal error, when we do not expect to have failures.
Here in rdb load its iether that we had bug in snapshot serializing json incorrect or user tries to load some json but we can not parse it.
I do think that its worth to have an error print here. Please also print the blob string here so it will be easier to investigate if we see this error.
How did you get to this error flow stefan?
There was a problem hiding this comment.
Yep I agree with Adi here, it would be useful for us to be aware of those problematic flows
There was a problem hiding this comment.
I didn't reproduce it, but I checked the memory tracking, and during the check, I found out that if an error occurs (so the JSON should be empty), we are still trying to use the parsed value
Signed-off-by: Stepan Bagritsevich <[email protected]>
Signed-off-by: Stepan Bagritsevich <[email protected]>
BagritsevichStepan
force-pushed
the
rdb_loader/fix-json-loading
branch
from
2ab51f8 to
1facbb4
Compare
Small fix for the case if we cannot parse json string during loading.