Skip to content

IcebergIterator._seek_to_usable_file swallows original error via bare raise Exception #5091

@mengw15

Description

@mengw15

What happened?

amber/src/main/python/core/storage/iceberg/iceberg_document.py:214-216:

except Exception:
    print("Could not read iceberg table:\n")
    raise Exception

Re-raises a bare Exception() — no args, no from. Callers see:

  • type(caught).__name__ == 'Exception' (the specific exception class is lost)
  • str(caught) == ''
  • caught.__cause__ is None (no explicit chaining)
  • The original error is preserved only via __context__ (Python's implicit context inside an except block)

Also: print writes to stdout instead of going through the logger.

Together this means any caller doing except Exception as e: log.error(str(e)) loses all diagnostic info when an iceberg-side failure (catalog auth, S3 IO, manifest corruption, etc.) occurs.

Fix: raise Exception(f"Could not read iceberg table: {e}") from e (or a bare raise to re-raise the original) and route the message through the project logger.

How to reproduce?

Drop the following into amber/src/main/python/core/storage/iceberg/test_iceberg_iterator_error_paths.py:

from unittest import mock
import pytest
from core.storage.iceberg import iceberg_document
from core.storage.iceberg.iceberg_document import IcebergIterator


class _FailingTable:
    def refresh(self):
        raise RuntimeError("Catalog auth failure: token expired")


def test_seek_to_usable_file_preserves_original_error():
    with mock.patch.object(
        iceberg_document,
        "load_table_metadata",
        return_value=_FailingTable(),
    ):
        it = IcebergIterator(0, None, None, "ns", "tbl", None, None)
        with pytest.raises(Exception) as exc_info:
            next(it)

    message_visible = "Catalog auth failure" in str(exc_info.value)
    cause_chained = exc_info.value.__cause__ is not None
    assert message_visible or cause_chained, (
        f"diagnostic lost: type={type(exc_info.value).__name__}, "
        f"str={str(exc_info.value)!r}, __cause__={exc_info.value.__cause__!r}"
    )

Current master output:

type='Exception', str='', __cause__=None,
__context__=RuntimeError('Catalog auth failure: token expired')

Test fails — top-level error has lost both the class and the message.

Version

1.1.0-incubating (Pre-release/Master)

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions