From 6a99d52a7ab743c11535d0ece3e51ae33193ce78 Mon Sep 17 00:00:00 2001 From: Amos Jun-yeung Ng Date: Sun, 22 Jan 2023 16:43:54 +0700 Subject: [PATCH 1/3] Only silently ignore expected errors I had to spend some time digging before I realized that serialization/deserialization errors were being swallowed here --- vcr/cassette.py | 4 ++-- vcr/persisters/filesystem.py | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/vcr/cassette.py b/vcr/cassette.py index 5822afac..bdbbaac9 100644 --- a/vcr/cassette.py +++ b/vcr/cassette.py @@ -11,7 +11,7 @@ from .errors import UnhandledHTTPRequestError from .matchers import get_matchers_results, method, requests_match, uri from .patch import CassettePatcherBuilder -from .persisters.filesystem import FilesystemPersister +from .persisters.filesystem import FilesystemPersister, ExpectedCassetteError from .record_mode import RecordMode from .serializers import yamlserializer from .util import partition_dict @@ -352,7 +352,7 @@ def _load(self): self.append(request, response) self.dirty = False self.rewound = True - except ValueError: + except ExpectedCassetteError: pass def __str__(self): diff --git a/vcr/persisters/filesystem.py b/vcr/persisters/filesystem.py index e9710638..7121341f 100644 --- a/vcr/persisters/filesystem.py +++ b/vcr/persisters/filesystem.py @@ -5,17 +5,21 @@ from ..serialize import deserialize, serialize +class ExpectedCassetteError(ValueError): + pass + + class FilesystemPersister: @classmethod def load_cassette(cls, cassette_path, serializer): cassette_path = Path(cassette_path) # if cassette path is already Path this is no operation if not cassette_path.is_file(): - raise ValueError("Cassette not found.") + raise ExpectedCassetteError("Cassette not found.") try: with cassette_path.open() as f: data = f.read() except UnicodeEncodeError as err: - raise ValueError("Can't read Cassette, Encoding is broken") from err + raise ExpectedCassetteError("Can't read Cassette, Encoding is broken") from err return deserialize(data, serializer) From 12d44d78e154aa646b66c82cdd518c776d2fe88c Mon Sep 17 00:00:00 2001 From: Amos Jun-yeung Ng Date: Tue, 20 Jun 2023 16:25:16 +1000 Subject: [PATCH 2/3] Split persister errors into CassetteNotFoundError and CassetteDecodeError --- docs/advanced.rst | 2 +- tests/integration/test_register_persister.py | 36 +++++++++++++++++++- tests/unit/test_cassettes.py | 13 +++++++ vcr/cassette.py | 4 +-- vcr/persisters/filesystem.py | 10 ++++-- 5 files changed, 58 insertions(+), 7 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index cf93dcc0..744d503c 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -136,7 +136,7 @@ Create your own persistence class, see the example below: Your custom persister must implement both ``load_cassette`` and ``save_cassette`` methods. The ``load_cassette`` method must return a deserialized cassette or raise -``ValueError`` if no cassette is found. +``CassetteNotFoundError`` if no cassette is found. Once the persister class is defined, register with VCR like so... diff --git a/tests/integration/test_register_persister.py b/tests/integration/test_register_persister.py index 1391b981..d8a71e81 100644 --- a/tests/integration/test_register_persister.py +++ b/tests/integration/test_register_persister.py @@ -5,9 +5,11 @@ import os from urllib.request import urlopen +import pytest + # Internal imports import vcr -from vcr.persisters.filesystem import FilesystemPersister +from vcr.persisters.filesystem import CassetteDecodeError, CassetteNotFoundError, FilesystemPersister class CustomFilesystemPersister: @@ -25,6 +27,19 @@ def save_cassette(cassette_path, cassette_dict, serializer): FilesystemPersister.save_cassette(cassette_path, cassette_dict, serializer) +class BadPersister(FilesystemPersister): + """A bad persister that raises different errors.""" + + @staticmethod + def load_cassette(cassette_path, serializer): + if "nonexistent" in cassette_path: + raise CassetteNotFoundError() + elif "encoding" in cassette_path: + raise CassetteDecodeError() + else: + raise ValueError("buggy persister") + + def test_save_cassette_with_custom_persister(tmpdir, httpbin): """Ensure you can save a cassette using custom persister""" my_vcr = vcr.VCR() @@ -53,3 +68,22 @@ def test_load_cassette_with_custom_persister(tmpdir, httpbin): with my_vcr.use_cassette(test_fixture, serializer="json"): response = urlopen(httpbin.url).read() assert b"difficult sometimes" in response + + +def test_load_cassette_with_custom_persister(tmpdir, httpbin): + """ + Ensure expected errors from persister are swallowed while unexpected ones + are passed up the stack. + """ + my_vcr = vcr.VCR() + my_vcr.register_persister(BadPersister) + + with my_vcr.use_cassette("bad/nonexistent") as cass: + assert len(cass) == 0 + + with my_vcr.use_cassette("bad/encoding") as cass: + assert len(cass) == 0 + + with pytest.raises(ValueError): + with my_vcr.use_cassette("bad/buggy") as cass: + pass diff --git a/tests/unit/test_cassettes.py b/tests/unit/test_cassettes.py index 41e3df53..d1352dbe 100644 --- a/tests/unit/test_cassettes.py +++ b/tests/unit/test_cassettes.py @@ -29,6 +29,19 @@ def test_cassette_load(tmpdir): assert len(a_cassette) == 1 +def test_cassette_load_nonexistent(): + a_cassette = Cassette.load(path="something/nonexistent.yml") + assert len(a_cassette) == 0 + + +def test_cassette_load_invalid_unicode(tmpdir): + a_file = tmpdir.join("invalid_unicode.yml") + with open(a_file, "wb") as fd: + fd.write(b"\xda") + with pytest.raises(UnicodeDecodeError): + Cassette.load(path=a_file) + + def test_cassette_not_played(): a = Cassette("test") assert not a.play_count diff --git a/vcr/cassette.py b/vcr/cassette.py index bdbbaac9..77ffe616 100644 --- a/vcr/cassette.py +++ b/vcr/cassette.py @@ -11,7 +11,7 @@ from .errors import UnhandledHTTPRequestError from .matchers import get_matchers_results, method, requests_match, uri from .patch import CassettePatcherBuilder -from .persisters.filesystem import FilesystemPersister, ExpectedCassetteError +from .persisters.filesystem import CassetteDecodeError, CassetteNotFoundError, FilesystemPersister from .record_mode import RecordMode from .serializers import yamlserializer from .util import partition_dict @@ -352,7 +352,7 @@ def _load(self): self.append(request, response) self.dirty = False self.rewound = True - except ExpectedCassetteError: + except (CassetteDecodeError, CassetteNotFoundError): pass def __str__(self): diff --git a/vcr/persisters/filesystem.py b/vcr/persisters/filesystem.py index 7121341f..b105fb24 100644 --- a/vcr/persisters/filesystem.py +++ b/vcr/persisters/filesystem.py @@ -5,7 +5,11 @@ from ..serialize import deserialize, serialize -class ExpectedCassetteError(ValueError): +class CassetteNotFoundError(FileNotFoundError): + pass + + +class CassetteDecodeError(ValueError): pass @@ -14,12 +18,12 @@ class FilesystemPersister: def load_cassette(cls, cassette_path, serializer): cassette_path = Path(cassette_path) # if cassette path is already Path this is no operation if not cassette_path.is_file(): - raise ExpectedCassetteError("Cassette not found.") + raise CassetteNotFoundError() try: with cassette_path.open() as f: data = f.read() except UnicodeEncodeError as err: - raise ExpectedCassetteError("Can't read Cassette, Encoding is broken") from err + raise CassetteDecodeError("Can't read Cassette, Encoding is broken") from err return deserialize(data, serializer) From 6d4df766ddff1e52238966c994bc460ef3f1f4b5 Mon Sep 17 00:00:00 2001 From: Amos Jun-yeung Ng Date: Fri, 23 Jun 2023 08:42:37 +0700 Subject: [PATCH 3/3] Address code review comments --- docs/advanced.rst | 3 ++- tests/integration/test_register_persister.py | 4 ++-- tests/unit/test_cassettes.py | 8 ++++---- vcr/persisters/filesystem.py | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index 744d503c..9cba3ba6 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -136,7 +136,8 @@ Create your own persistence class, see the example below: Your custom persister must implement both ``load_cassette`` and ``save_cassette`` methods. The ``load_cassette`` method must return a deserialized cassette or raise -``CassetteNotFoundError`` if no cassette is found. +either ``CassetteNotFoundError`` if no cassette is found, or ``CassetteDecodeError`` +if the cassette cannot be successfully deserialized. Once the persister class is defined, register with VCR like so... diff --git a/tests/integration/test_register_persister.py b/tests/integration/test_register_persister.py index d8a71e81..42b8736b 100644 --- a/tests/integration/test_register_persister.py +++ b/tests/integration/test_register_persister.py @@ -70,10 +70,10 @@ def test_load_cassette_with_custom_persister(tmpdir, httpbin): assert b"difficult sometimes" in response -def test_load_cassette_with_custom_persister(tmpdir, httpbin): +def test_load_cassette_persister_exception_handling(tmpdir, httpbin): """ Ensure expected errors from persister are swallowed while unexpected ones - are passed up the stack. + are passed up the call stack. """ my_vcr = vcr.VCR() my_vcr.register_persister(BadPersister) diff --git a/tests/unit/test_cassettes.py b/tests/unit/test_cassettes.py index d1352dbe..cbe9de1a 100644 --- a/tests/unit/test_cassettes.py +++ b/tests/unit/test_cassettes.py @@ -34,12 +34,12 @@ def test_cassette_load_nonexistent(): assert len(a_cassette) == 0 -def test_cassette_load_invalid_unicode(tmpdir): - a_file = tmpdir.join("invalid_unicode.yml") +def test_cassette_load_invalid_encoding(tmpdir): + a_file = tmpdir.join("invalid_encoding.yml") with open(a_file, "wb") as fd: fd.write(b"\xda") - with pytest.raises(UnicodeDecodeError): - Cassette.load(path=a_file) + a_cassette = Cassette.load(path=str(a_file)) + assert len(a_cassette) == 0 def test_cassette_not_played(): diff --git a/vcr/persisters/filesystem.py b/vcr/persisters/filesystem.py index b105fb24..d7bd4518 100644 --- a/vcr/persisters/filesystem.py +++ b/vcr/persisters/filesystem.py @@ -22,7 +22,7 @@ def load_cassette(cls, cassette_path, serializer): try: with cassette_path.open() as f: data = f.read() - except UnicodeEncodeError as err: + except UnicodeDecodeError as err: raise CassetteDecodeError("Can't read Cassette, Encoding is broken") from err return deserialize(data, serializer)