[BLOCKING] [R] Two R methods discards errors from C API functions

[hcho3]

C API functions indicate errors by their return values. All callers of C API functions should check the return value and handle error if the return value is -1. There are two places in R-package/src/xgboost_R.cc where this is not done:

xgboost/R-package/src/xgboost_R.cc

Lines 376 to 381 in a4de2f6

	SEXP XGBoosterLoadJsonConfig_R(SEXP handle, SEXP value) {
	R_API_BEGIN();
	XGBoosterLoadJsonConfig(R_ExternalPtrAddr(handle), CHAR(asChar(value)));
	R_API_END();
	return R_NilValue;
	}

xgboost/R-package/src/xgboost_R.cc

Lines 398 to 405 in a4de2f6

	SEXP XGBoosterUnserializeFromBuffer_R(SEXP handle, SEXP raw) {
	R_API_BEGIN();
	XGBoosterUnserializeFromBuffer(R_ExternalPtrAddr(handle),
	RAW(raw),
	length(raw));
	R_API_END();
	return R_NilValue;
	}

These function calls should have been wrapped with CHECK_CALL() macro to handle errors. Currently, errors from these two functions are silently discarded.

In #5794, the error message was cryptic

  [06:35:16] amalgamation/../src/learner.cc:506: Check failed: mparam_.num_feature != 0 (0 vs. 0) : 0 feature is supplied.  Are you using raw Booster interface?
Stack trace:
  [bt] (0) /home/redacted/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(dmlc::LogMessageFatal::~LogMessageFatal()+0x64) [0x7f2da04ae154]
  [bt] (1) /home/redacted/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(xgboost::LearnerConfiguration::ConfigureNumFeatures()+0xf0) [0x7f2da054d220]
  [bt] (2) /home/redacted/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(xgboost::LearnerConfiguration::Configure()+0x356) [0x7f2da05aaba6]
  [bt] (3) /home/redacted/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(xgboost::LearnerImpl::Predict(std::shared_ptr<xgboost::DMatrix>, bool, xgboost::HostDeviceVector<float>*, unsigned int, bool, bool, bool, bool, bool)+0x7d) [0x7f2da0569e1d]
  [bt] (4) /home/redacted/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(XGBoosterPredict+0xf0) [0x7f2da04c27e0]
  [bt] (5) /home/redacted/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(XGBoosterPredict_R+0x89) [0x7f2da04a9799]
  [bt] (6) /opt/R/3.6.3/lib/R/lib/libR.so(+0x131bdd) [0x7f2dcf047bdd]
  [bt] (7) /opt/R/3.6.3/lib/R/lib/libR.so(Rf_eval+0x380) [0x7f2dcf051580]
  [bt] (8) /opt/R/3.6.3/lib/R/lib/libR.so(+0x13d121) [0x7f2dcf053121]

This message is misleading, as it sounds as if the error had occurred in the prediction function. In fact, the real error occurred earlier, when the model was loaded. When I inserted CHECK_CALL() macro in R-package/src/xgboost_R.cc, I got the correct error message:

Error in xgb.unserialize(modelfile) : 
  [03:19:04] ../src/learner.cc:864: Check failed: header == serialisation_header_: 

  If you are loading a serialized model (like pickle in Python) generated by older
  XGBoost, please export the model by calling `Booster.save_model` from that version
  first, then load it back in current version.  There's a simple script for helping
  the process. See:

    https://xgboost.readthedocs.io/en/latest/tutorials/saving_model.html

  for reference to the script, and more details about differences between saving model and
  serializing.


Stack trace:
  [bt] (0) /home/phcho/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(dmlc::LogMessageFatal::~LogMessageFatal()+0x7c) [0x7f1f596f737c]
  [bt] (1) /home/phcho/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(xgboost::LearnerIO::Load(dmlc::Stream*)+0x81b) [0x7f1f5980bc3b]
  [bt] (2) /home/phcho/R/x86_64-pc-linux-gnu-library/3.6/xgboost/libs/xgboost.so(XGBoosterUnserializeFromBuffer+0x8c) [0x7f1f596faaec]

[hcho3]

I will write a pull request to fix this issue soon. I'd like to back-port the pull request to 1.1.x branch.