ToolHandle: throw exception if retrieval fails

If a ToolHandle is retrieved with the wrong interface, an StatusCode::FAILURE is returned, but no error message is displayed. This is especially bad when ToolHandles are automatically retrieved by their parent at the end of initialize - initialization will fail with no obvious cause.

This MR addresses this by having ToolHandle::retrieve throw an exception if either the retrieval from the ToolSvc fails, or if the dcast to the required iface fails.

I'm a bit torn about this however. We've gone from returning a StatusCode::FAILURE, which can easily be dealt with at runtime, to throwing an exception, which requires significantly different behaviour from the client. I don't know if there is a real use case for continuing execution if the retrieval fails. I think the best solution would have been to print an error message, and return a FAILURE as before, but there's no easy way to get hold of the MessageSvc from within the ToolHandle.

• [2018-01-25 21:58] Validation started with lhcb-gaudi-merge#396
• [2018-01-26 00:05] Validation started with lhcb-gaudi-head-py3#45
• [2018-01-26 00:05] Validation started with lhcb-tdr-test#38
• [2018-01-26 00:07] Validation started with lhcb-gaudi-head#1735
• [2018-01-26 08:43] Validation started with lhcb-gaudi-head#1736
• [2018-01-27 00:04] Validation started with lhcb-gaudi-head-py3#46
• [2018-01-27 00:06] Validation started with lhcb-tdr-test#39
• [2018-01-27 00:08] Validation started with lhcb-gaudi-head#1737
• [2018-01-28 00:05] Validation started with lhcb-gaudi-head-py3#47
• [2018-01-28 00:06] Validation started with lhcb-tdr-test#40
• [2018-01-28 00:08] Validation started with lhcb-gaudi-head#1738
• [2018-01-29 00:04] Validation started with lhcb-gaudi-head-py3#48
• [2018-01-29 00:05] Validation started with lhcb-tdr-test#41
• [2018-01-29 00:09] Validation started with lhcb-gaudi-head#1739
• [2018-01-30 00:05] Validation started with lhcb-gaudi-head-py3#49
• [2018-01-30 00:07] Validation started with lhcb-tdr-test#42
• [2018-01-30 00:17] Validation started with lhcb-gaudi-head#1740
• [2018-01-30 01:48] Validation started with lhcb-tdr-test#42
• [2018-01-31 00:04] Validation started with lhcb-gaudi-head-py3#50
• [2018-01-31 00:05] Validation started with lhcb-tdr-test#43
• [2018-01-31 00:07] Validation started with lhcb-gaudi-head#1741
• [2018-02-01 00:04] Validation started with lhcb-gaudi-head-py3#51
• [2018-02-01 00:07] Validation started with lhcb-tdr-test#44
• [2018-02-01 00:11] Validation started with lhcb-gaudi-head#1742
• [2018-02-01 10:00] Validation started with lhcb-gaudi-head#1743
• [2018-02-01 11:37] Validation started with lhcb-tdr-test#45
• [2018-02-02 00:06] Validation started with lhcb-gaudi-head-py3#52
• [2018-02-02 00:06] Validation started with lhcb-tdr-test#46
• [2018-02-02 00:07] Validation started with lhcb-gaudi-head#1744
• [2018-02-02 11:52] Validation started with lhcb-upgrade-hackathon#1
• [2018-02-02 11:57] Validation started with lhcb-upgrade-hackathon#2
• [2018-02-02 15:45] Validation started with lhcb-upgrade-hackathon#3
• [2018-02-02 15:58] Validation started with lhcb-upgrade-hackathon#4
• [2018-02-02 17:32] Validation started with lhcb-upgrade-hackathon#5
• [2018-02-02 21:03] Validation started with lhcb-upgrade-hackathon#6
• [2018-02-03 00:04] Validation started with lhcb-gaudi-head-py3#53
• [2018-02-03 00:06] Validation started with lhcb-tdr-test#47
• [2018-02-03 00:06] Validation started with lhcb-upgrade-hackathon#7
• [2018-02-03 00:08] Validation started with lhcb-gaudi-head#1745
• [2018-02-04 00:04] Validation started with lhcb-gaudi-head-py3#54
• [2018-02-04 00:05] Validation started with lhcb-upgrade-hackathon#8
• [2018-02-04 00:06] Validation started with lhcb-tdr-test#48
• [2018-02-04 00:08] Validation started with lhcb-gaudi-head#1746
• [2018-02-05 00:05] Validation started with lhcb-gaudi-head-py3#55
• [2018-02-05 00:06] Validation started with lhcb-upgrade-hackathon#9
• [2018-02-05 00:09] Validation started with lhcb-tdr-test#49
• [2018-02-05 00:09] Validation started with lhcb-gaudi-head#1747
• [2018-02-05 09:40] Validation started with lhcb-upgrade-hackathon#10
• [2018-02-05 09:42] Validation started with lhcb-tdr-test#50
• [2018-02-05 09:52] Validation started with lhcb-gaudi-head#1748
• [2018-02-06 00:03] Validation started with lhcb-gaudi-head-py3#56
• [2018-02-06 00:06] Validation started with lhcb-tdr-test#51
• [2018-02-06 00:08] Validation started with lhcb-gaudi-head#1749
• [2018-02-07 00:03] Validation started with lhcb-gaudi-head-py3#57
• [2018-02-07 00:04] Validation started with lhcb-tdr-test#52
• [2018-02-07 00:08] Validation started with lhcb-gaudi-head#1750
• [2018-02-08 00:04] Validation started with lhcb-gaudi-head-py3#58
• [2018-02-08 00:07] Validation started with lhcb-tdr-test#53
• [2018-02-08 00:09] Validation started with lhcb-gaudi-head#1751
• [2018-02-08 13:42] Validation started with lhcb-gaudi-head#1752
• [2018-02-09 00:05] Validation started with lhcb-gaudi-head-py3#59
• [2018-02-09 00:07] Validation started with lhcb-tdr-test#54
• [2018-02-09 00:12] Validation started with lhcb-gaudi-head#1753

approved this merge request

Concerning your hesitation, for me the choice between return codes and exceptions is guided by the following factors:

• Is the error the product of a bug in the code, or a relatively expected event (e.g. cannot open a file due to a permission mismatch)? In the former case, I always go for exceptions, since a bug is by definition unexpected and may only be recovered from at a very high level (e.g. by restarting the faulty process). In the later case, return values that make it more explicit in the API that an error must be handled can be useful.
• Do you expect the error to occur often? From a performance point of view, exceptions deal better with infrequent events, whereas special error return values deal better with frequent events.

I would spontaneously expect ToolHandle retrieval failures to be the result of a bug (e.g. framework misconfiguration) and to occur rarely. If so, exceptions sound like the better choice. But you tell me if the underlying expectation is correct :)

mentioned in issue #10

I can create some scenarios where one would loop over say a bunch of different tracking Tools in a ToolHandleArray, which all have the same base class, but are derived in different ways, and try retrieving them until a specific derived class is found. But I have no idea is this is a realistic scenario....

Usually it's an error.

We could add a StatusCode tryRetrieve() which could be used for optional tools.

assigned to @clemenci

changed milestone to %v30r2

added 1 commit

• 608e39be - moved throw to i_retrieve, added test in GaudiExamples

Compare with previous version

added 1 commit

• 12137965 - fix formatting

Compare with previous version

added 1 commit

• 3cf3c22e - rolling back some changes

Compare with previous version

resolved all discussions

I rolled back some changes so that if a Tool retrieval from the ToolSvc returns a failure, it propagates this back up instead of throwing an exception. This is in order to maintain existing behavior.

I will open a new MR that will change the behavior a little more radically, where all retrievals will throw an exception if there's an error, and will also add a StatusCode tryRetrieve() method that does not throw. The question with that though, is what sort of side effects should a tryRetrieve have? Should it do the retrieve and just trap the exceptions? Should it reset the m_pObject to its original state after it tries? If a Tool actually needs to be created as part of the retrieval, should it create it, and delete it when it's done, or leave it there?

approved this merge request

merged

mentioned in commit b52f650d

added C++ framework label