Ticket #6512 (closed: fixed)
[ICAT] Algorithm error reporting
Reported by: | Owen Arnold | Owned by: | Jay Rainey |
---|---|---|---|
Priority: | major | Milestone: | Release 3.1 |
Component: | Framework | Keywords: | ICAT |
Cc: | Blocked By: | #6490, #8445 | |
Blocking: | Tester: | Alex Buts |
Description (last modified by Nick Draper) (diff)
A lot of algorithms in the ICAT category are throwing the same basic exception message.
throw std::runtime_error("Error when getting the catalog information from the Facilities.xml file.");
Some of these were replaced as part of #6490 to give better information on login. It would be best if someone where to go through these error scenarios and check that
- They are reporting the right information, which should be useful to the end-user
- That there is reduced duplication of error messages. Looks like a lot of copy-paste at the moment.
The quick fix #6490 should also be reviewed, because the throwCatalogError() function is duplicated across two algorithms.
Nick. Reassign as you see appropriate. It might be that I'm the best person to look at this.
Change History
comment:4 Changed 7 years ago by Nick Draper
- Milestone changed from Release 2.6 to Backlog
Moved to backlog at the code freeze for R2.6
comment:5 Changed 7 years ago by Nick Draper
- Owner changed from Nick Draper to Jay Rainey
- Description modified (diff)
- Milestone changed from Backlog to Release 3.0
comment:7 Changed 7 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
comment:8 Changed 7 years ago by Jay Rainey
- Status changed from verify to reopened
- Resolution fixed deleted
comment:11 Changed 7 years ago by Jay Rainey
- Summary changed from Error Reporting in ICAT to [ICAT] Algorithm error reporting
comment:13 Changed 7 years ago by Jay Rainey
- Blocked By 8322 removed
There has been no changes made in this ticket. However, I have made changes in #8322 that address this ticket so you can test with a develop build.
To test
- Open Mantid, and click Help -> First time setup and change your facility to something other than ISIS.
- Attempt to log into the catalog and you will receive an error message explaining why.
Ensure the changes in #8322 address the error reporting in Mantid ensuring that there is no longer a duplication of error messages across algorithms.
comment:14 Changed 7 years ago by Jay Rainey
- Status changed from reopened to verify
- Resolution set to fixed
comment:15 Changed 7 years ago by Roman Tolchenov
- Status changed from verify to verifying
- Tester set to Roman Tolchenov
comment:16 Changed 7 years ago by Roman Tolchenov
- Status changed from verifying to reopened
- Resolution fixed deleted
Algorithms CatalogListInstruments, CatalogListInvestigationTypes, CatalogMyDataSearch catch all runtime errors and throw another one with the same message "Please login..." regardless of the cause of the caught exception and discarding its message. I think the exceptions need to be differentiated here, for example have a specialized LoginError.
I seems that ICat4Catalog.h is missing in CMakeLists.txt.
comment:17 Changed 7 years ago by Jay Rainey
- Status changed from reopened to verify
- Resolution set to fixed
I have removed the related try/catch in #8445 as they are no longer needed, and added the header file in #8446.
To test this ticket download & install a develop build and ensure changes in #8322 address this ticket, as described in comment:13.
comment:18 Changed 7 years ago by Nick Draper
- Status changed from verify to verifying
- Tester changed from Roman Tolchenov to Nick Draper
comment:19 Changed 7 years ago by Nick Draper
- Status changed from verifying to verify
- Tester Nick Draper deleted
Too many things going on at the same time
comment:20 Changed 7 years ago by Owen Arnold
- Status changed from verify to verifying
- Tester set to Owen Arnold
comment:21 Changed 7 years ago by Owen Arnold
- Status changed from verifying to reopened
- Resolution fixed deleted
- Blocked By 8445 added
comment:22 Changed 7 years ago by Owen Arnold
Since this ticket is now dependent on 8445. Set that as the blocker for this.
comment:23 Changed 7 years ago by Jay Rainey
- Status changed from reopened to verify
- Resolution set to fixed
comment:24 Changed 7 years ago by Alex Buts
- Status changed from verify to verifying
- Tester changed from Owen Arnold to Alex Buts
No such error exists in the code any more.
otherwise, seems works fine.
comment:26 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 7358
Moved to r2.6 at the end of r2.5