Context Navigation

← Previous Ticket
Next Ticket →

Ticket #8936 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Uncaught exception on Results Table tab of MuonAnalysis

Reported by:	Arturs Bekasovs	Owned by:	Arturs Bekasovs
Priority:	critical	Milestone:	Release 3.1.1
Component:	Muon	Keywords:	PatchCandidate
Cc:		Blocked By:
Blocking:		Tester:	Martyn Gigg

Description

When the ADS contains a fitted workspace which wasn't created by MuonAnalysis and thus doesn't have (even basic) log values copied to it, the exception is thrown.

To reproduce:

Load MUSR00015189 to MantidPlot.
Fit any of the loaded group members using the standard MantidPlot fit functionality.
Open MuonAnalysis and go to the Results Table tab. The exception is thrown.

Change History

comment:1 Changed 7 years ago by Nick Draper

Status changed from new to assigned

Bulk move of tickets out of triage (new) to assigned at the introduction of the triage state

comment:2 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Add a more fitted workspace filtering checks.

Changeset: 0dc84cf031f643e642e81e6d49c82189101e95e6

comment:3 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Make _Parameters a constant.

Changeset: 2efbd0245acb295861c676a277ae2fc43cbc9e29

comment:4 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Add check for corresponding _Parameters table.

Changeset: c3157f80a9bf3a04386d2f3d04cf78972fd066de

comment:5 Changed 7 years ago by Arturs Bekasovs

Refs #8936. A few minor improvements.

Changeset: 54ed2d1c0d95736647561574cc7bbf0a2748eb01

comment:6 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Safer check for _Parameters workspace..

Changeset: 8d2c2313740e59b512ccffae43bdb4526c855132

comment:7 Changed 7 years ago by Arturs Bekasovs

Tester:

Check that described scenario isn't leading to exception any more. The results list should just be empty.

Try to come up with a few other simple use cases for when Results table might break. E.g. if you accidentally delete one of the _Parameters workspaces or you have a non-fitted workspace which ends with _Workspace. There will always be a way to mess up with ADS which breaks it, I just want it to survive the simplest misuse scenarios.

comment:8 Changed 7 years ago by Arturs Bekasovs

Status changed from assigned to inprogress

comment:9 Changed 7 years ago by Arturs Bekasovs

Status changed from inprogress to verify
Resolution set to fixed

comment:10 Changed 7 years ago by Samuel Jackson

Status changed from verify to verifying
Tester set to Samuel Jackson

comment:11 Changed 7 years ago by Samuel Jackson

Status changed from verifying to reopened
Resolution fixed deleted

As discussed, there's a couple of places where you're getting a workspace without checking that it exists. Understandably there are use cases where it's impossible to do anything about this other than throw an error, but showing something other than the top level exception would be better.

comment:12 Changed 7 years ago by Arturs Bekasovs

Keywords PatchCandidate added

Very easy to reproduce.

comment:13 Changed 7 years ago by Nick Draper

Milestone changed from Release 3.2 to Release 3.1.1

Moved to patch release 3.1.1

comment:14 Changed 7 years ago by Arturs Bekasovs

Status changed from reopened to inprogress

Refs #8936. Check for null on every retrieve.

If workspace is not found/is of incorrect type - NotFoundError is thrown.

Changeset: d3de946c37611dc790a4f7046ef4f2a315f4f11b

comment:15 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Unify workspace checking code.

Changeset: 7dd19d34f5fa2142e43040fe0c04bb3ff7d441ad

comment:16 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Catch exceptions from table creation.

Changeset: ead9d6b38ef7356c88a40e1313c4d72b0a251caf

comment:17 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Improve checking method.

Changeset: 58386bba7c4a59fe3edfaa1424d00cd5352cfc4f

comment:18 Changed 7 years ago by Arturs Bekasovs

Refs #8936. Refresh tables if not found on creation.

Changeset: 99503838065bab99d6d97fe6c2c23c158b8302d2

comment:19 Changed 7 years ago by Arturs Bekasovs

Every retrieve operation has a null-check now, so at least we should never segfault.

Speaking about exception handling, there are two cases when exceptions can be thrown:

Populating the GUI tables.
Creating the Results TableWorkspace.

For the first one, I am not catching any exceptions myself. That's because the list of workspaces is checked for validity before being passed to table-population routine. If something bad happens even after that - that's really an unexpected exception and my own catching routine wouldn't be able to do anything better than the standard one.

For the second one, it is expected that user might delete some of the workspaces and invalidate the list. So here I am catching exceptions, reporting appropriate errors and refreshing the list of workspaces so that the second try will succeed.

Ideally, the whole thing should be re-factored to retrieve workspaces in one place only and then store retrieved values, in order to minimize the necessity of such checks. Additionally, ADS delete events could be caught, causing tables to be refreshed. I've created maintenance ticket for that: #9080.

Tester:

In addition to previous test description, please check that what's described above works:

Workspaces are filtered. When Result Table tab is opened, the list should contain valid fitted workspaces only.

Deleting/invalidating the workspace while on the tab and then trying to create the table should cause an appropriate error to be shown and tables to be refreshed.

Last edited 7 years ago by Arturs Bekasovs (previous) (diff)