root [skip-ci] Raise exception in TFile constructor when remote path is passed

The public TFile constructor cannot be used for files that must be read through remote protocols. This is a common source of confusion for users.

I split the error in two commits, one for the C++ constructor and one for its pythonization. I would like to discuss:

The wording of the error message
The usage of std::exception in the C++ side. This should be a better practice in general, but the rest of TFile uses the TObject::Error method for this kind of problems. The downside of that is that it doesn't really stop the execution of the program

With the current status, the errors would look like this

>>> import ROOT
>>> f = ROOT.TFile("https://root.cern/files/tutorials/hsimple.root")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/vpadulan/programs/rootproject/rootinstall/tfile-constructor-error-distrdf-release/lib/ROOT/_pythonization/_tfile.py", line 88, in _TFileConstructor
    raise ValueError("Cannot handle path to remote file '{}' in TFile constructor. Use TFile::Open instead.".format(args[0]))
ValueError: Cannot handle path to remote file 'https://root.cern/files/tutorials/hsimple.root' in TFile constructor. Use TFile::Open instead.

$: root.exe
   ------------------------------------------------------------------
  | Welcome to ROOT 6.27/01                        https://root.cern |
  | (c) 1995-2022, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for linuxx8664gcc on Jul 27 2022, 19:14:18                 |
  | From heads/master@v6-25-02-1893-ge1d4a59786                      |
  | With c++ (GCC) 12.1.1 20220507 (Red Hat 12.1.1-1)                |
  | Try '.help'/'.?', '.demo', '.license', '.credits', '.quit'/'.q'  |
   ------------------------------------------------------------------

root [0] TFile f{"https://root.cern/files/tutorials/hsimple.root"};
Error in <TRint::HandleTermInput()>: std::invalid_argument caught: Cannot handle path to remote file 'https://root.cern/files/tutorials/hsimple.root' in TFile constructor. Use TFile::Open instead

$: ./test.o
terminate called after throwing an instance of 'std::invalid_argument'
  what():  Cannot handle path to remote file 'https://root.cern/files/tutorials/hsimple.root' in TFile constructor. Use TFile::Open instead.
Aborted (core dumped)

Jul 27 '22 20:07 vepadulano

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac1015/cxx17, mac11/cxx14, windows10/cxx14 How to customize builds

Jul 27 '22 20:07 phsft-bot

Build failed on ROOT-debian10-i386/soversion. Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build See console output.

Failing tests:

And 20 more

Jul 27 '22 20:07 phsft-bot

Build failed on ROOT-ubuntu18.04/nortcxxmod. Running on sft-ubuntu-1804-2.cern.ch:/build/workspace/root-pullrequests-build See console output.

Failing tests:

And 30 more

Jul 27 '22 20:07 phsft-bot

Build failed on ROOT-performance-centos8-multicore/cxx17. Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build See console output.

Failing tests:

And 101 more

Jul 27 '22 20:07 phsft-bot

Build failed on ROOT-ubuntu2004/python3. Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build See console output.

Failing tests:

And 32 more

Jul 27 '22 20:07 phsft-bot

Build failed on windows10/cxx14. Running on null:C:\build\workspace\root-pullrequests-build See console output.

Failing tests:

And 3 more

Jul 27 '22 21:07 phsft-bot

Build failed on mac11/cxx14. Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build See console output.

Failing tests:

And 100 more

Jul 27 '22 21:07 phsft-bot

Build failed on mac1015/cxx17. Running on macitois21.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build See console output.

Failing tests:

And 32 more

Jul 27 '22 22:07 phsft-bot

You probably guessed it already - but the issue is that derived classes use TFile's public constructors, too, and so while the purpose of derived classes is to handle remote protocols, TFile's constructors will nonetheless be called with such a "filename". Yes - that's not super well designed (we should have had a common base class, and TFile should be an incarnation of those, not be the common base itself), but we cannot change that at this point.

As this is in the ctor you cannot even check "am I really a TFile or a derived class? because the object's vtable isn't populated enough yet, and thus a runtime-type-info "who are you?" will just say "TFile`!" even for derived classes.

We would have to introduce protected ctors instead, and change all derived classes to use those.

Jul 28 '22 17:07 Axel-Naumann

Indeed, I should have updated the PR with a comment earlier. At least we can raise the error on the python side though, the call to the pythonization happens before the C++ constructor is called. Maybe the check can be something more specific like

url = ROOT.TUrl(filename)
if url.GetProtocol() != "file":
    raise ValueError()

instead of the current more simplistic check

Jul 28 '22 17:07 vepadulano

Closing this pending better solution

Aug 18 '22 08:08 vepadulano

root root copied to clipboard

[skip-ci] Raise exception in TFile constructor when remote path is passed

Failing tests:

Failing tests:

Failing tests:

Failing tests:

Failing tests:

Failing tests:

Failing tests:

root
root copied to clipboard