BUG: close hdf store on any error

In `read_hdf`, if the `store.select()` call throws either a
`ValueError`, a `TypeError` or a `KeyError` then the store is closed.

However, if any other exception is thrown (e.g. an `AssertionError` if
there are gaps in blocks ref_loc) , the store is not closed and some
client code could end up trying to reopen the store and hit an
error like: `the file XXX is already opened. Please close it before
reopening in write mode`. Furthermore, the exception is re-raised in all
cases.

This commit just closes store in all cases.

Closes #28430

Author: Adrien Di Mascio

doc/source/whatsnew/v1.0.0.rst

@@ -727,6 +727,7 @@ I/O
 - Bug in :meth:`DataFrame.to_clipboard` which did not work reliably in ipython (:issue:`22707`)
 - Bug in :func:`read_json` where default encoding was not set to ``utf-8`` (:issue:`29565`)
 - Bug in :class:`PythonParser` where str and bytes were being mixed when dealing with the decimal field (:issue:`29650`)
+- :meth:`read_hdf` now closes the store on any error thrown (:issue:`28430`)
 -
 
 Plotting

pandas/io/pytables.py

@@ -426,15 +426,12 @@ def read_hdf(
             chunksize=chunksize,
             auto_close=auto_close,
         )
-    except (ValueError, TypeError, KeyError):
-        if not isinstance(path_or_buf, HDFStore):
-            # if there is an error, close the store if we opened it.
-            try:
-                store.close()
-            except AttributeError:
-                pass
-
-        raise
+    finally:
+        # if there is an error, close the store if we opened it.
+        try:
+            store.close()
+        except AttributeError:
+            pass
 
 
 def _is_metadata_of(group: "Node", parent_group: "Node") -> bool:

pandas/tests/io/pytables/test_store.py

@@ -447,6 +447,25 @@ def check_default_mode():
         check("w")
         check_default_mode()
 
+    def test_store_closed_on_error(self, mocker):
+        """check hdf store is closed whichever error occurs"""
+        mocker.patch("pandas.io.pytables.HDFStore.select").side_effect = AssertionError(
+            "Gaps in blk ref_locs"
+        )
+        df = tm.makeDataFrame()
+        with ensure_clean_path(self.path) as path:
+            df.to_hdf(path, "df")
+            try:
+                pd.read_hdf(path, "df")
+            except AssertionError:  # this is expected, because of our mock
+                pass
+            try:
+                HDFStore(path, mode="w")
+            except ValueError as exc:
+                pytest.fail(
+                    "store not closed properly, got error {exc}".format(exc=exc)
+                )
+
     def test_reopen_handle(self, setup_path):
 
         with ensure_clean_path(setup_path) as path: