diff --git a/orgui/app/QScanSelector.py b/orgui/app/QScanSelector.py index 379e322..becebde 100644 --- a/orgui/app/QScanSelector.py +++ b/orgui/app/QScanSelector.py @@ -36,7 +36,6 @@ from silx.gui import qt -from silx.gui.hdf5.Hdf5TreeModel import Hdf5TreeModel from silx.gui import icons from silx.gui.plot.AlphaSlider import NamedImageAlphaSlider # from silx.gui.widgets import HorizontalSliderWithBrowser @@ -107,7 +106,7 @@ def __init__(self, parentmainwindow, parent=None): self.hdfTreeView = silx.gui.hdf5.Hdf5TreeView(self) self.hdfTreeView.setSortingEnabled(True) self.hdfTreeView.addContextMenuCallback(self.nexus_treeview_callback) - self.hdf5model = Hdf5TreeModel(self.hdfTreeView, ownFiles=True) + self.hdf5model = qutils.RobustHdf5TreeModel(self.hdfTreeView, ownFiles=True) self.hdfTreeView.setModel(self.hdf5model) self.hdfTreeView.setExpandsOnDoubleClick(False) self.hdf5model.setFileMoveEnabled(True) @@ -1139,7 +1138,24 @@ def _onCloseFile(self): objects = list(self.hdfTreeView.selectedH5Nodes()) if len(objects) > 0: obj = objects[0] - self.hdf5model.removeH5pyObject(obj.file) + try: + self.hdf5model.removeH5pyObject(obj.file) + except Exception as e: + # reading a file on a disconnected drive raises in h5py, which + # must not terminate orGUI, see issue #23. + logger.error( + "Cannot close the data file properly.", + exc_info=True, + extra={ + "title": "Cannot close file", + "description": f"Cannot close the data file properly:\n{e}\n" + "The file might be corrupted. If it is still listed, " + "use 'refresh file' to rebuild the file list.", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) def nexus_treeview_callback(self, event): objects = list(event.source().selectedH5Nodes()) @@ -1147,7 +1163,7 @@ def nexus_treeview_callback(self, event): obj = objects[0] # for single selection menu = event.menu() action = qt.QAction("Refresh", menu) - action.triggered.connect(lambda: self.hdf5model.synchronizeH5pyObject(obj)) + action.triggered.connect(lambda: self._synchronizeH5pyObject(obj)) # menu.addAction(action) # if obj.ntype is h5py.Dataset: action = qt.QAction("display data", menu) @@ -1162,7 +1178,24 @@ def _onRefreshFileOld(self): objects = list(self.hdfTreeView.selectedH5Nodes()) if len(objects) > 0: obj = objects[0] + self._synchronizeH5pyObject(obj) + + def _synchronizeH5pyObject(self, obj): + """Reload a data file in the tree view and report errors.""" + try: self.hdf5model.synchronizeH5pyObject(obj) + except Exception as e: + logger.error( + "Cannot refresh the data file.", + exc_info=True, + extra={ + "title": "Cannot refresh file", + "description": f"Cannot refresh the data file:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) def __getRelativePath(self, model, rootIndex, index): """Returns a relative path from an index to his rootIndex. @@ -1185,11 +1218,28 @@ def __getRelativePath(self, model, rootIndex, index): def _onRefreshFile(self): qt.QApplication.setOverrideCursor(qt.Qt.WaitCursor) + try: + self._refreshFileTreeView() + except Exception as e: + # rebuilding the tree view reads all open files, which fails if + # one of them became unavailable. + logger.error( + "Cannot refresh the file tree view.", + exc_info=True, + extra={ + "title": "Cannot refresh file", + "description": f"Cannot refresh the file tree view:\n{e}", + "show_dialog": True, + "parent": self, + }, + ) + finally: + qt.QApplication.restoreOverrideCursor() + def _refreshFileTreeView(self): selection = self.hdfTreeView.selectionModel() indexes = selection.selectedIndexes() if indexes == []: - qt.QApplication.restoreOverrideCursor() qt.QMessageBox.warning( self, "No file selected", "Cannot refresh file: No file selected." ) @@ -1247,17 +1297,30 @@ def _onRefreshFile(self): modelnew = self.hdfTreeView.findHdf5TreeModel() for h5, filename in h5files: - modelnew.insertFile(filename, 0) + try: + modelnew.insertFile(filename, 0) + except Exception as e: + logger.error( + "Cannot reload the data file %s.", + filename, + exc_info=True, + extra={ + "title": "Cannot reload file", + "description": f"Cannot reload the data file {filename}:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) # self.__expandNodesFromPaths(self.hdfTreeView, index, paths) # self.hdf5model.appendFile(filename) - qt.QApplication.restoreOverrideCursor() def createTreeView(self): self.hdfTreeView = silx.gui.hdf5.Hdf5TreeView(self) self.hdfTreeView.setSortingEnabled(True) self.hdfTreeView.addContextMenuCallback(self.nexus_treeview_callback) - self.hdf5model = Hdf5TreeModel(self.hdfTreeView, ownFiles=True) + self.hdf5model = qutils.RobustHdf5TreeModel(self.hdfTreeView, ownFiles=True) self.hdfTreeView.setModel(self.hdf5model) self.hdfTreeView.setExpandsOnDoubleClick(False) self.hdf5model.setFileMoveEnabled(True) diff --git a/orgui/app/database.py b/orgui/app/database.py index 3d02683..2a20b96 100644 --- a/orgui/app/database.py +++ b/orgui/app/database.py @@ -57,6 +57,12 @@ class DBCloseError(IOError): pass +class DBUnavailableError(IOError): + """Raised if an operation requires a database file, but none is open.""" + + pass + + DEFAULT_FILTERS = { # Filters available with h5py/libhdf5 "Raw": None, "GZip": "gzip", @@ -168,9 +174,23 @@ def __init__(self, plot, parent=None): self.view.doubleClicked.connect(self._onNEXUSDoubleClicked) self.view.addContextMenuCallback(self.nexus_treeview_callback) - self.temp_directory = tempfile.TemporaryDirectory(dir=os.path.join(os.getcwd())) - tempfilepath = os.path.join(self.temp_directory.name, "orgui_database.h5") - self.createNewDBFile(tempfilepath) + try: + self.openTemporaryDatabase() + except Exception: + # orGUI must start up even without a writable location for the + # scratch database, e.g. if the drive with the working directory + # is disconnected or read only. + logger.error( + "Cannot create the initial temporary database.", + exc_info=True, + extra={ + "title": "Cannot create database", + "description": "Cannot create the initial temporary database. " + "Create a new database before saving any data.", + "show_dialog": True, + "parent": self, + }, + ) # self.add_nxdict(gauss) # self.add_nxdict(gauss) self.setCentralWidget(self.view) @@ -199,16 +219,31 @@ def __init__(self, plot, parent=None): self.addToolBar(toolbar) - def _onNEXUSDoubleClicked(self, index): # ToDo add try except with popup message! - nodes = list(self.view.selectedH5Nodes()) - if len(nodes) > 0: - obj = nodes[0] - if obj.ntype is h5py.Dataset: - roi_node = self.get_roinode(obj.h5py_object) - if roi_node is not None: - self.plot_signal_callback(roi_node, obj) - return - self.plot_default(obj.h5py_object) + def _onNEXUSDoubleClicked(self, index): + try: + nodes = list(self.view.selectedH5Nodes()) + if len(nodes) > 0: + obj = nodes[0] + if obj.ntype is h5py.Dataset: + roi_node = self.get_roinode(obj.h5py_object) + if roi_node is not None: + self.plot_signal_callback(roi_node, obj) + return + self.plot_default(obj.h5py_object) + except Exception as e: + # reading the database can fail at any time, e.g. if the drive + # with the database was disconnected. + logger.error( + "Cannot read the selected database entry.", + exc_info=True, + extra={ + "title": "Cannot read database", + "description": f"Cannot read the selected database entry:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) def plot_default(self, h5py_object): try: @@ -249,11 +284,35 @@ def get_scannode(self, obj): obj = obj.parent def nexus_treeview_callback(self, event): + """Fill the context menu of the database tree view. + + Building the menu reads attributes from the database file, which can + fail at any time, e.g. if the drive with the database was + disconnected. Such errors must not escape into the Qt event loop. + """ + try: + self._fillNexusTreeviewMenu(event) + except Exception as e: + logger.error( + "Cannot create the context menu of the database entry.", + exc_info=True, + extra={ + "title": "Cannot read database", + "description": f"Cannot read the selected database entry:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) + + def _fillNexusTreeviewMenu(self, event): objects = list(event.source().selectedH5Nodes()) + if not objects: + return obj = objects[0] # for single selection menu = event.menu() action = qt.QAction("Refresh", menu) - action.triggered.connect(lambda: self.hdf5model.synchronizeH5pyObject(obj)) + action.triggered.connect(lambda: self.onRefreshNode(obj)) menu.addAction(action) action = qt.QAction("details", menu) action.triggered.connect(lambda: self.view_data_callback(obj)) @@ -282,7 +341,7 @@ def nexus_treeview_callback(self, event): menu.addAction(action) menu.addSeparator() action = qt.QAction("delete", menu) - action.triggered.connect(lambda: self.delete_node(obj.h5py_object)) + action.triggered.connect(lambda: self.onDeleteNode(obj.h5py_object)) menu.addAction(action) elif meta and "rocking" in meta: @@ -294,7 +353,7 @@ def nexus_treeview_callback(self, event): menu.addAction(action) menu.addSeparator() action = qt.QAction("delete", menu) - action.triggered.connect(lambda: self.delete_node(obj.h5py_object)) + action.triggered.connect(lambda: self.onDeleteNode(obj.h5py_object)) menu.addAction(action) if meta and "scan" in meta: @@ -388,6 +447,8 @@ def onSaveNewDBFile(self): "parent": self, }, ) + finally: + self._ensureDatabaseAvailable() def onNewDatabase(self): fileTypeDict = {"NEXUS Files (*.h5)": ".h5", "All files (*)": ""} @@ -435,6 +496,8 @@ def onNewDatabase(self): "parent": self, }, ) + finally: + self._ensureDatabaseAvailable() def onSaveDBFile(self): fileTypeDict = {"NEXUS Files (*.h5)": ".h5", "All files (*)": ""} @@ -490,25 +553,21 @@ def onOpenDatabase(self): "parent": self, }, ) + finally: + self._ensureDatabaseAvailable() def saveNewDBFile(self, filename): - alldata = nxtodict(self.nxfile) + alldata = nxtodict(self._requireOpenFile()) self.createNewDBFile(filename, alldata) def saveDBFile(self, filename): - alldata = nxtodict(self.nxfile) + alldata = nxtodict(self._requireOpenFile()) dicttonx( alldata, filename, create_dataset_args={"compression": self.compression} ) def createNewDBFile(self, filename, datadict=None): - if self.nxfile is not None: - try: - self.close() - except Exception as e: - raise DBCloseError( - "Cannot close previous database file.\nThe database might be corrupted." # noqa: E501 - ) from e # convert to common IOError since can also be RuntimeError + self._closePreviousDBFile() fileattrs = { "@NX_class": "NXroot", @@ -527,13 +586,7 @@ def createNewDBFile(self, filename, datadict=None): self.openDBFile(filename) def openDBFile(self, filename): - if self.nxfile is not None: - try: - self.close() - except Exception as e: - raise DBCloseError( - "Cannot close previous database file.\nThe database might be corrupted." # noqa: E501 - ) from e # convert to common IOError since can also be RuntimeError + self._closePreviousDBFile() self.nxfile = silx.io.h5py_utils.File(filename, "a") self._filepath = filename while self.hdf5model.hasPendingOperations(): @@ -542,12 +595,120 @@ def openDBFile(self, filename): self.hdf5model.insertH5pyObject(self.nxfile) self.view.expandToDepth(0) - def add_nxdict(self, nxentry, update_mode="add", h5path="/"): + def _closePreviousDBFile(self): + """Close the currently open database file, if there is any. + + :raises DBCloseError: + If the database file cannot be closed properly. The database is + detached from orGUI in any case, so that a new database file can + be created or opened afterwards. + """ if self.nxfile is None: - raise Exception("No database file open.") + return + try: + self.close() + except Exception as e: + raise DBCloseError( + "Cannot close previous database file.\nThe database might be corrupted." # noqa: E501 + ) from e # convert to common IOError since can also be RuntimeError + + def isOpen(self): + """Whether a usable database file is currently open. + + Returns False if the database was never opened, or if the file handle + was closed, e.g. after a failed write to a disconnected drive. + + :rtype: bool + """ + return bool(self.nxfile) + + def _requireOpenFile(self): + """Return the open database file. + + :raises DBUnavailableError: If no database file is open. + """ + if not self.isOpen(): + raise DBUnavailableError( + "No database file open. Create or open a database first." + ) + return self.nxfile + + def openTemporaryDatabase(self): + """Create and open a scratch database in a temporary directory. + + Used at startup and as a fallback if the database on disk became + unavailable, so that orGUI stays operational. + """ + # close first: this discards the previous temporary directory + self._closePreviousDBFile() + self.temp_directory = self._createTempDirectory() + self.createNewDBFile( + os.path.join(self.temp_directory.name, "orgui_database.h5") + ) + + @staticmethod + def _createTempDirectory(): + """Create the temporary directory of the scratch database. + + The working directory is preferred, since it is usually located on the + same drive as the measured data. If it is not writable, e.g. because + the drive was disconnected, the system temporary directory is used. + """ + try: + return tempfile.TemporaryDirectory( + dir=os.getcwd(), ignore_cleanup_errors=True + ) + except OSError: + logger.warning( + "Cannot create a temporary database directory in %s. " + "Using the system temporary directory instead.", + os.getcwd(), + exc_info=True, + ) + return tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + + def _ensureDatabaseAvailable(self): + """Fall back to a temporary database if no database file is open. + + Called after a failed attempt to create or open a database file, so + that data can still be saved somewhere and orGUI keeps operating. + """ + if self.isOpen(): + return + try: + self.openTemporaryDatabase() + except Exception: + logger.error( + "Cannot create a temporary database.", + exc_info=True, + extra={ + "title": "No database available", + "description": "Cannot create a temporary database. " + "No data can be saved until a new database is created.", + "show_dialog": True, + "parent": self, + }, + ) + return + logger.warning( + "No database file available, created the temporary database %s.", + self._filepath, + extra={ + "title": "Temporary database created", + "description": "No database file is available. Created the " + f"temporary database\n{self._filepath}\n" + "Save the database to a permanent location to keep the data.", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) + + def add_nxdict(self, nxentry, update_mode="add", h5path="/"): + nxfile = self._requireOpenFile() dicttonx( nxentry, - self.nxfile, + nxfile, h5path=h5path, update_mode="add", create_dataset_args={"compression": self.compression}, @@ -555,14 +716,14 @@ def add_nxdict(self, nxentry, update_mode="add", h5path="/"): while self.hdf5model.hasPendingOperations(): qt.QApplication.processEvents() time.sleep(0.01) - self.hdf5model.synchronizeH5pyObject(self.nxfile) + self.hdf5model.synchronizeH5pyObject(nxfile) self.view.expandToDepth(0) def write_scan_config(self, scan_name, config=None): """Write the default config group for a scan.""" if config is None: config = ConfigData.from_gui(self.config_target) - group = self.nxfile[scan_name] + group = self._requireOpenFile()[scan_name] self.config_handler.create_dataset_args = {"compression": self.compression} return self.config_handler.write_scan_config(group, config) @@ -570,7 +731,7 @@ def write_integration_config(self, integration_path, config=None): """Write the config group used for an integration result.""" if config is None: config = ConfigData.from_gui(self.config_target) - group = self.nxfile[integration_path] + group = self._requireOpenFile()[integration_path] self.config_handler.create_dataset_args = {"compression": self.compression} return self.config_handler.write_integration_config(group, config) @@ -583,9 +744,64 @@ def onDeleteScan(self, obj): ), # noqa: E501 ) if btn == qt.QMessageBox.Yes: + self.onDeleteNode(obj) + + def onDeleteNode(self, obj): + """Delete a node from the database and report errors to the user.""" + try: self.delete_node(obj) + except Exception as e: + logger.error( + "Cannot delete entry from the database.", + exc_info=True, + extra={ + "title": "Cannot delete database entry", + "description": f"Cannot delete entry from the database:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) + + def onRefreshNode(self, obj): + """Reload a database entry in the tree view, reporting errors.""" + try: + self.hdf5model.synchronizeH5pyObject(obj) + except Exception as e: + logger.error( + "Cannot refresh the database entry.", + exc_info=True, + extra={ + "title": "Cannot refresh database entry", + "description": f"Cannot refresh the database entry:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) def onShowRoIntegrate(self, obj): + try: + h5_obj = self._findRockingScanGroup(obj) + except Exception as e: + logger.error( + "Cannot read the rocking scan from the database.", + exc_info=True, + extra={ + "title": "Cannot read database", + "description": f"Cannot read the rocking scan:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) + return + if h5_obj is None: + return # invalid dataset + self.sigChangeRockingScan.emit(h5_obj.name) + + def _findRockingScanGroup(self, obj): + """Return the rocking scan group of a database entry, or None.""" meta = obj.h5py_object.attrs.get("orgui_meta", False) h5_obj = obj.h5py_object while h5_obj.name != "/": # search for rocking scan group @@ -603,19 +819,19 @@ def onShowRoIntegrate(self, obj): self, ) msgbox.exec() - return # invalid dataset + return None # invalid dataset - dir_name = h5_obj.name - self.sigChangeRockingScan.emit(dir_name) + return h5_obj def delete_node(self, obj): + nxfile = self._requireOpenFile() basename = obj.name.split("/")[-1] objpar = obj.parent del objpar[basename] while self.hdf5model.hasPendingOperations(): qt.QApplication.processEvents() time.sleep(0.01) - self.hdf5model.synchronizeH5pyObject(self.nxfile) + self.hdf5model.synchronizeH5pyObject(nxfile) self.view.expandToDepth(0) def onRenameNode(self, obj): @@ -628,52 +844,153 @@ def onRenameNode(self, obj): basename, ) if success and newname != "": - self.rename_node(obj, newname) + try: + self.rename_node(obj, newname) + except Exception as e: + logger.error( + "Cannot rename entry in the database.", + exc_info=True, + extra={ + "title": "Cannot rename database entry", + "description": f"Cannot rename entry {basename}:\n{e}", + "show_dialog": True, + "dialog_level": logging.WARNING, + "parent": self, + }, + ) def rename_node(self, obj, newname): + nxfile = self._requireOpenFile() basename = obj.name.split("/")[-1] objpar = obj.parent objpar.move(basename, newname) while self.hdf5model.hasPendingOperations(): qt.QApplication.processEvents() time.sleep(0.01) - self.hdf5model.synchronizeH5pyObject(self.nxfile) + self.hdf5model.synchronizeH5pyObject(nxfile) self.view.expandToDepth(0) def close(self): - if self.nxfile is not None: - timer = 1 - if self.hdf5model.hasPendingOperations(): - qt.QApplication.processEvents() - while self.hdf5model.hasPendingOperations() and timer < 6000: - time.sleep(0.01) - if not (timer % 1000): - qt.QApplication.processEvents() - timer += 1 - if timer == 6000: - raise Exception( - "Timeout on hdf5 model operation, This is probably a bug, or a very long writing operation occurs, please report if this is a long writing opertion" # noqa: E501 - ) - self.hdf5model.removeH5pyObject(self.nxfile) - timer = 1 - while self.hdf5model.hasPendingOperations() and timer < 6000: - time.sleep(0.01) - if not (timer % 1000): - qt.QApplication.processEvents() - timer += 1 - if timer == 6000: - raise Exception( - "Timeout on hdf5 model operation, This is probably a bug, or a very long writing operation occurs, please report if this is a long writing opertion" # noqa: E501 - ) + """Close the database file and detach it from the tree view. + + orGUI always detaches from the database file, even if it cannot be + closed properly, e.g. if the drive with the database was disconnected. + A new database can therefore be created or opened afterwards, also + after this method raised an error. + + :raises DBCloseError: + If pending database operations do not finish in time. The database + file is not closed in this case, since closing it while the tree + model reads it can crash the application. + :raises Exception: + The error raised by h5py if the file cannot be closed properly, + usually a RuntimeError or an OSError. The database file might be + corrupted in this case. + """ + if self.nxfile is None: + return + # keep the file open on timeout, the tree model might still read it. + self._waitForPendingOperations() + + nxfile = self.nxfile + self.nxfile = None # the file handle must not be used any longer + try: + self._detachFromTreeView(nxfile) + self._waitForPendingOperations() try: - self.nxfile.close() - except RuntimeError: - self.nxfile = None + nxfile.close() + except Exception: logger.error( "Closing of database file failed. The database file might be corrupted!", # noqa: E501 exc_info=True, ) raise + finally: + self._discardTempDirectory() + + def closeSafe(self, show_dialog=False): + """Close the database file, without raising on error. + + Used on shutdown paths, where a failed close must not escape into the + Qt event loop or the exception hook. + + :param bool show_dialog: + Notify the user with a message box if the database file could not + be closed properly. + :returns: True if the database file was closed properly. + :rtype: bool + """ + try: + self.close() + except Exception: + # logged as a warning on purpose: in cli context, error records + # re-raise the exception, see orgui.logger_utils. + logger.warning( + "Cannot close the database file. The database might be corrupted!", + exc_info=True, + extra={ + "title": "The database might be corrupted", + "description": "Cannot close the database file properly. " + "The data written since the last save might be lost.", + "show_dialog": show_dialog, + "dialog_level": logging.ERROR, + "parent": self, + }, + ) + return False + return True + + def _waitForPendingOperations(self, timeout=60.0): + """Wait until the tree model finished its asynchronous operations. - if hasattr(self, "temp_directory"): - del self.temp_directory + :param float timeout: Maximum waiting time in seconds. + :raises DBCloseError: If the operations did not finish in time. + """ + deadline = time.monotonic() + timeout + while self.hdf5model.hasPendingOperations(): + if time.monotonic() > deadline: + raise DBCloseError( + "Timeout on hdf5 model operation. This is probably a bug, or a " + "very long writing operation occurs. Please report this if no " + "long writing operation is running." + ) + # the pending operations of the tree model only finish if Qt can + # process events, see also add_nxdict. + qt.QApplication.processEvents() + time.sleep(0.01) + + def _detachFromTreeView(self, nxfile): + """Remove a database file from the tree view. Does not raise. + + Removing the item reads from the file, which can fail if the file + became unavailable. orGUI has to detach from it in any case. + """ + # logged as warnings on purpose: in cli context, error records re-raise + # the exception, see orgui.logger_utils. + try: + self.hdf5model.removeH5pyObject(nxfile) + except Exception: + logger.warning( + "Cannot remove the database from the tree view. " + "Clearing the tree view instead.", + exc_info=True, + ) + try: + self.hdf5model.clear() + except Exception: + logger.warning("Cannot clear the database tree view.", exc_info=True) + + def _discardTempDirectory(self): + """Delete the temporary directory of the scratch database, if any.""" + temp_directory = getattr(self, "temp_directory", None) + if temp_directory is None: + return + del self.temp_directory + try: + temp_directory.cleanup() + except Exception: + logger.warning( + "Cannot remove the temporary database directory %s.", + temp_directory.name, + exc_info=True, + ) diff --git a/orgui/app/orGUI.py b/orgui/app/orGUI.py index 1497146..6da0b27 100644 --- a/orgui/app/orGUI.py +++ b/orgui/app/orGUI.py @@ -272,7 +272,7 @@ def __init__(self, configfile, parent=None): self.scanSelector.sigROIChanged.connect(self.updateROI) self.scanSelector.sigROIChanged.connect(self._onROITrackingChanged) - self.scanSelector.sigROIintegrate.connect(self.integrateROI) + self.scanSelector.sigROIintegrate.connect(self._onIntegrateROI) self.scanSelector.sigSearchHKL.connect(self.onSearchHKLforStaticROI) self.scanSelector.excludeImageAct.toggled.connect(self._onToggleExcludeImage) @@ -1308,6 +1308,70 @@ def rocking_static_extraction(self, xy, hsize, vsize): ro_name = "rocking_static" return self.rocking_integrate(xy, roi_keys, hkl_del_gam, refldict, ro_name) + def _onIntegrateROI(self): + """GUI-only: integrate the active ROI and report unexpected errors. + + The integration reads the scan images and writes into the database. + Both can fail at any time, e.g. if a drive is disconnected during the + integration. Such an error must not terminate orGUI. + + :returns: Status dictionary of :meth:`integrateROI`. + :rtype: dict + """ + try: + return self.integrateROI() + except Exception as e: + logger.error( + "Error during integration.", + exc_info=True, + extra={ + "title": "Error during integration", + "description": f"Error during integration:\n{e}\n" + "The integration was aborted.", + "show_dialog": True, + "parent": self, + }, + ) + return { + "status": "error", + "message": "Error during integration", + "traceback": traceback.format_exc(), + } + + def _saveIntegrationResult(self, nxdict, description): + """Write an integration result into the database. + + Writing can fail at any time, e.g. if the drive with the database was + disconnected during the integration. This must be reported to the + user instead of terminating orGUI. + + :param dict nxdict: NeXus data to add to the database. + :param str description: Description of the data, used in messages. + :returns: None on success, an error status dictionary otherwise. + :rtype: dict or None + """ + try: + self.database.add_nxdict(nxdict) + except Exception as e: + logger.error( + "Cannot save the integrated data into the database.", + exc_info=True, + extra={ + "title": "Cannot save integrated data", + "description": f"Cannot save {description} into the " + f"database:\n{e}\nCreate a new database at an available " + "location and integrate the scan again.", + "show_dialog": True, + "parent": self, + }, + ) + return { + "status": "error", + "message": "Cannot save the integrated data into the database", + "traceback": traceback.format_exc(), + } + return None + def rocking_integrate(self, xylist, rois, hkl_del_gam, refldict, name): """Integrate rocking-scan images over prepared ROIs. @@ -1353,8 +1417,8 @@ def rocking_integrate(self, xylist, rois, hkl_del_gam, refldict, name): "message": "No image found in current scan", "traceback": traceback.format_exc(), } - if self.database.nxfile is None: - logger.exception( + if not self.database.isOpen(): + logger.error( "Cannot integrate scan: No database available.", extra={ "title": "Cannot integrate scan", @@ -2227,7 +2291,11 @@ def sumImage(i): data_2d_structured[self.activescanname]["measurement"][name]["rois"] = rois - self.database.add_nxdict(data_2d_structured) + error = self._saveIntegrationResult( + data_2d_structured, f"the rocking scan integration {name}" + ) + if error is not None: + return error logger.info(f"Rocking integration succeeded and data saved with name {name}") return {"status": "success"} @@ -4700,8 +4768,8 @@ def integrateROI(self): "message": "No image found in current scan", "traceback": traceback.format_exc(), } - if self.database.nxfile is None: - logger.exception( + if not self.database.isOpen(): + logger.error( "Cannot perform stationary scan integration: no database available.", extra={ "title": "Cannot integrate scan", @@ -5567,7 +5635,11 @@ def sumImage(i): names_to_log += availname2 - self.database.add_nxdict(data) + error = self._saveIntegrationResult( + data, f"the scan integration {names_to_log}" + ) + if error is not None: + return error logger.info(f"stationary scan integrated and saved with name(s) {names_to_log}") return {"status": "success"} @@ -5840,8 +5912,12 @@ def _centerTrackedROI(self, image_no): ) def closeEvent(self, event): - """GUI/CLI hint: close the database before the main window closes.""" - self.database.close() + """GUI/CLI hint: close the database before the main window closes. + + A database file which cannot be closed properly, e.g. on a + disconnected drive, must not prevent orGUI from closing. + """ + self.database.closeSafe(show_dialog=True) super().closeEvent(event) @@ -6383,7 +6459,8 @@ def exception_hook(self, exc_type, exc_value, exc_traceback): "Cannot save database.", traceback.format_exc(), ) - self.orgui.database.close() + # must not raise again inside the exception hook + self.orgui.database.closeSafe() else: print("No QApplication instance available.") sys.exit(1) diff --git a/orgui/app/qutils.py b/orgui/app/qutils.py index 304eb17..5052924 100644 --- a/orgui/app/qutils.py +++ b/orgui/app/qutils.py @@ -28,10 +28,42 @@ __maintainer__ = "Timo Fuchs" __email__ = "tfuchs@cornell.edu" +import logging + from silx.gui import qt from silx.gui import icons +from silx.gui.hdf5.Hdf5TreeModel import Hdf5TreeModel import numpy as np +logger = logging.getLogger(__name__) + + +class RobustHdf5TreeModel(Hdf5TreeModel): + """HDF5 tree model which tolerates files that cannot be closed. + + :class:`Hdf5TreeModel` closes the files it owns when the corresponding + item is removed from the model. h5py raises if the file cannot be written + on close, e.g. if the drive holding the file was disconnected. Such an + error aborts the removal of the item and propagates into the Qt event + loop, which terminates orGUI (issue #23). + + This model logs the error instead and removes the item, so that the user + can continue to work with the remaining files. + """ + + def _closeFileIfOwned(self, node): + try: + super()._closeFileIfOwned(node) + except Exception: + filename = getattr(node.obj, "filename", "") + # logged as a warning on purpose: in cli context, error records + # re-raise the exception, see orgui.logger_utils. + logger.warning( + "Closing of file %s failed. The file might be corrupted!", + filename, + exc_info=True, + ) + def messagebox_detailed_message( parent, title, text, detailed_text, icon, buttons=qt.QMessageBox.Ok diff --git a/orgui/app/test/test_database.py b/orgui/app/test/test_database.py new file mode 100644 index 0000000..291c50d --- /dev/null +++ b/orgui/app/test/test_database.py @@ -0,0 +1,121 @@ +"""Tests for the resilience of the orGUI database against I/O errors. + +The database file can become unavailable at any time, e.g. if the drive +holding it is disconnected (issue #23). orGUI must report this and keep +operating instead of terminating. +""" + +import os + +import h5py +import pytest +from silx.gui import qt + +from orgui.app.database import DataBase, DBUnavailableError +from orgui.app.qutils import RobustHdf5TreeModel + + +_QAPP = None + + +def _getQApplication(): + """Return the application instance, keeping a reference to it.""" + global _QAPP + _QAPP = qt.QApplication.instance() or qt.QApplication([]) + return _QAPP + + +def _writeError(*args, **kwargs): + raise RuntimeError( + "Can't decrement id ref count (file write failed: errno = 22, " + "error message = 'Invalid argument')" + ) + + +@pytest.fixture +def database(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + _getQApplication() + db = DataBase(None) + yield db + db.closeSafe() + # the widget is not deleted on purpose: silx parents its global wait icon + # and thread pool to the first widget using them, deleting the widget + # breaks the tree model of all following tests. + + +def test_temporary_database_is_open_after_creation(database): + assert database.isOpen() + assert os.path.isfile(database._filepath) + + +def test_close_does_not_keep_a_stale_file_handle(database): + database.close() + + assert not database.isOpen() + with pytest.raises(DBUnavailableError): + database.add_nxdict({"@NX_class": "NXcollection", "counter": 1}) + + +def test_failed_close_detaches_and_allows_a_new_database(database, tmp_path): + nxfile = database.nxfile + nxfile.close = _writeError # simulate a disconnected drive + + with pytest.raises(Exception): + database.close() + + # orGUI must not hold on to the broken file, otherwise no new database + # can be created afterwards. + assert not database.isOpen() + assert database.hdf5model.rowCount() == 0 + + database.createNewDBFile(str(tmp_path / "new_database.h5")) + assert database.isOpen() + database.add_nxdict({"@NX_class": "NXcollection", "counter": 1}, h5path="/entry") + assert "entry/counter" in database.nxfile + + +def test_close_safe_reports_failure_without_raising(database): + database.nxfile.close = _writeError + + assert database.closeSafe() is False + assert not database.isOpen() + assert database.closeSafe() is True # nothing left to close + + +def test_fallback_database_is_created_if_the_new_database_fails(database, monkeypatch): + monkeypatch.setattr( + qt.QFileDialog, + "getSaveFileName", + staticmethod( + lambda *args, **kwargs: ( + os.path.join(os.getcwd(), "missing_drive", "database.h5"), + "", + ) + ), + ) + + database.onNewDatabase() # must not raise, the directory does not exist + + # a temporary database keeps orGUI operational + assert database.isOpen() + database.add_nxdict({"@NX_class": "NXcollection", "counter": 1}, h5path="/entry") + + +def test_robust_tree_model_removes_file_which_cannot_be_closed(tmp_path): + _getQApplication() + filename = str(tmp_path / "data.h5") + with h5py.File(filename, "w") as h5: + h5["counter"] = 1 + + model = RobustHdf5TreeModel(ownFiles=True) + model.insertFile(filename) + assert model.rowCount() == 1 + + h5file = model.data( + model.index(0, 0), role=RobustHdf5TreeModel.H5PY_OBJECT_ROLE + ).file + h5file.close = _writeError + + model.removeH5pyObject(h5file) # must not raise into the Qt event loop + assert model.rowCount() == 0 diff --git a/orgui/main.py b/orgui/main.py index 1384527..89e15fb 100644 --- a/orgui/main.py +++ b/orgui/main.py @@ -323,7 +323,7 @@ def out_prompt_tokens(self): logger.info("loading orGUI") mainWindow = orGUI(configfile) mainWindow.numberthreads = ncpu - app.aboutToQuit.connect(mainWindow.database.close) + app.aboutToQuit.connect(mainWindow.database.closeSafe) namespace = {"app": app, "orgui": mainWindow, "ub": mainWindow.ubcalc} @@ -332,14 +332,14 @@ def out_prompt_tokens(self): try: namespace = runpy.run_path(options.input, init_globals=namespace) except Exception: - mainWindow.database.close() + mainWindow.database.closeSafe() app.quit() raise logger.info(f"Completed batch script {options.input}") if not options.keeprunning: logger.info("All tasks completed. Application will now exit.") - mainWindow.database.close() + mainWindow.database.closeSafe() app.quit() return @@ -348,7 +348,7 @@ def out_prompt_tokens(self): try: ipshell(local_ns=namespace) finally: - mainWindow.database.close() + mainWindow.database.closeSafe() app.quit() else: raise Exception(f"{options.configfile} is no file") @@ -406,7 +406,7 @@ def _start_GUI(options, ncpu): qr = mainWindow.frameGeometry() qr.moveCenter(current_screen.geometry().center()) mainWindow.move(qr.topLeft()) - app.aboutToQuit.connect(mainWindow.database.close) + app.aboutToQuit.connect(mainWindow.database.closeSafe) namespace = {"app": app, "orgui": mainWindow, "ub": mainWindow.ubcalc} if options.input: logger.info(f"Run batch script {options.input}") @@ -417,7 +417,7 @@ def _start_GUI(options, ncpu): logger.info(f"Completed batch script {options.input}") if not options.keeprunning: logger.info("All tasks completed. Application will now exit.") - mainWindow.database.close() + mainWindow.database.closeSafe() app.quit() return if hasattr(mainWindow, "console_dockwidget"):