From cae1d68c7dcb1cc6d7c98a5c7778169ac0cfdb99 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:07:39 +0200 Subject: [PATCH 01/11] fix: Let a missing image URL raise ImageURLNotFound in base crawlers The three shared base crawlers asserted that the parser had found an image URL. CrawlerImage.url is deliberately optional, and add_image() validates it into an ImageURLNotFound, which is classified as CrawlerBroken and logged as an error naming the comic and date. The assert preempted that with a bare AssertionError, which fell through to the catch-all handler, and would vanish entirely under python -O. --- src/comics/aggregator/crawler.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/comics/aggregator/crawler.py b/src/comics/aggregator/crawler.py index a7ebc0be..0c4f53c2 100644 --- a/src/comics/aggregator/crawler.py +++ b/src/comics/aggregator/crawler.py @@ -315,7 +315,6 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: url = page.src('img[id="theComicImage"]') if not url: url = page.content('meta[property="og:image"]') - assert url return CrawlerImage(url) @@ -361,7 +360,6 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: if release["release"] == pub_date.strftime("%Y-%m-%d"): page = self.parse_page(release["url"]) url = page.src('img[itemprop="image"]') - assert url return CrawlerImage(url) return None @@ -384,7 +382,6 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: for entry in feed.for_date(pub_date): page = self.parse_page(entry.link) url = page.src("img#cc-comic") - assert url text = page.title("img#cc-comic") title = re.sub(r".+? - (.+)", r"\1", entry.title) From 3757726f3632bd74226a994623d91a367ac202ed Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:08:51 +0200 Subject: [PATCH 02/11] refactor: Remove the unraisable DoesNotExist from LxmlParser Nothing in the codebase raises DoesNotExist, so the handler wrapping _get_all() could never run. Both the handler and the exception class go. --- src/comics/aggregator/lxmlparser.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/comics/aggregator/lxmlparser.py b/src/comics/aggregator/lxmlparser.py index e3d6f8bd..88a7dc14 100644 --- a/src/comics/aggregator/lxmlparser.py +++ b/src/comics/aggregator/lxmlparser.py @@ -235,14 +235,11 @@ def _get_one( return value def _get_all(self, attr: str, selector: str) -> list[str]: - try: - return [ - self._decode(value) - for el in self._select_all(selector) - if (value := el.text_content() if attr == "text" else el.get(attr)) - ] - except DoesNotExist: - return [] + return [ + self._decode(value) + for el in self._select_all(selector) + if (value := el.text_content() if attr == "text" else el.get(attr)) + ] def _select_one(self, selector: str) -> HtmlElement | None: match self.root.cssselect(selector): @@ -287,9 +284,5 @@ class LxmlParserException(ComicsError): pass -class DoesNotExist(LxmlParserException): - pass - - class MultipleElementsReturned(LxmlParserException): pass From edf04fc275acca87022d7199f51001746071e1fd Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:09:20 +0200 Subject: [PATCH 03/11] refactor: Remove the unreachable _decode() from LxmlParser lxml returns str from both Element.get() and text_content(), so the bytes branch could never run. That it was applied in _get_all() but not in _get_one() showed nothing depended on it either way. --- src/comics/aggregator/lxmlparser.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/comics/aggregator/lxmlparser.py b/src/comics/aggregator/lxmlparser.py index 88a7dc14..31e905eb 100644 --- a/src/comics/aggregator/lxmlparser.py +++ b/src/comics/aggregator/lxmlparser.py @@ -236,7 +236,7 @@ def _get_one( def _get_all(self, attr: str, selector: str) -> list[str]: return [ - self._decode(value) + value for el in self._select_all(selector) if (value := el.text_content() if attr == "text" else el.get(attr)) ] @@ -271,14 +271,6 @@ def _parse_string(self, value: str | bytes) -> HtmlElement: value = "" return fromstring(value) - def _decode(self, value: str | bytes) -> str: - if isinstance(value, bytes): - try: - return value.decode("utf-8") - except UnicodeDecodeError: - return value.decode("iso-8859-1") - return value - class LxmlParserException(ComicsError): pass From a0c7fadfc3e0841acd18cc59d65ca0024ad8c22c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:09:40 +0200 Subject: [PATCH 04/11] fix: Correct the return type of LxmlParser.text() The implementation was annotated as returning list[str] | str | None, while both overloads and _get_one() only ever produce str | None. --- src/comics/aggregator/lxmlparser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/comics/aggregator/lxmlparser.py b/src/comics/aggregator/lxmlparser.py index 31e905eb..a5be5b0b 100644 --- a/src/comics/aggregator/lxmlparser.py +++ b/src/comics/aggregator/lxmlparser.py @@ -179,14 +179,14 @@ def contents(self, selector: str) -> list[str]: def text(self, selector: str, *, default: str) -> str: ... @overload - def text(self, selector: str, *, default: str | None = ...) -> str | None: ... + def text(self, selector: str, *, default: str | None = None) -> str | None: ... def text( self, selector: str, *, default: str | None = None, - ) -> list[str] | str | None: + ) -> str | None: """Return the text contained by the element matching `selector`.""" return self._get_one("text", selector, default=default) From 263f10020385bd065342624f16645550826f5a82 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:10:56 +0200 Subject: [PATCH 05/11] feat: Add first=True to the singular LxmlParser accessors The singular accessors raise MultipleElementsReturned when a selector matches more than one element. Pages that legitimately match several, where the comic is the first one, had to fall back to the plural accessor and index into it, guarding against the empty list by hand. first=True takes the first match in document order instead of raising, so those call sites collapse to the ordinary singular form. --- src/comics/aggregator/lxmlparser.py | 227 +++++++++++++++++++---- src/comics/comics/anleggsplassen.py | 7 +- src/comics/comics/cyanideandhappiness.py | 6 +- src/comics/comics/gucomics.py | 4 +- src/comics/comics/joyoftech.py | 10 +- src/comics/comics/spaceavalanche.py | 5 +- src/comics/comics/thisishistorictimes.py | 6 +- src/comics/comics/wulffmorgenthaler.py | 6 +- 8 files changed, 213 insertions(+), 58 deletions(-) diff --git a/src/comics/aggregator/lxmlparser.py b/src/comics/aggregator/lxmlparser.py index a5be5b0b..d044d49e 100644 --- a/src/comics/aggregator/lxmlparser.py +++ b/src/comics/aggregator/lxmlparser.py @@ -31,6 +31,10 @@ class LxmlParser: - Plural methods, e.g. [`srcs()`][comics.aggregator.lxmlparser.LxmlParser.srcs], return a list of zero or more values. + + Pass `first=True` to a singular method to take the first match in + document order instead of raising, for pages that legitimately match + several elements. """ _retrieved_url: str | None @@ -52,77 +56,174 @@ def __init__( raise LxmlParserException("Parser needs URL or string to operate on") @overload - def href(self, selector: str, *, default: str) -> str: ... + def href( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def href(self, selector: str, *, default: str | None = None) -> str | None: ... + def href( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... - def href(self, selector: str, *, default: str | None = None) -> str | None: + def href( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: """Return the `href` attribute of the element matching `selector`.""" - return self._get_one("href", selector, default=default) + return self._get_one("href", selector, default=default, first=first) def hrefs(self, selector: str) -> list[str]: """Return the `href` attribute of the elements matching `selector`.""" return self._get_all("href", selector) @overload - def src(self, selector: str, *, default: str) -> str: ... + def src( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def src(self, selector: str, *, default: str | None = None) -> str | None: ... + def src( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... - def src(self, selector: str, *, default: str | None = None) -> str | None: + def src( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: """Return the `src` attribute of the element matching `selector`.""" - return self._get_one("src", selector, default=default) + return self._get_one("src", selector, default=default, first=first) def srcs(self, selector: str) -> list[str]: """Return the `src` attribute of the elements matching `selector`.""" return self._get_all("src", selector) @overload - def alt(self, selector: str, *, default: str) -> str: ... + def alt( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def alt(self, selector: str, *, default: str | None = None) -> str | None: ... + def alt( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... - def alt(self, selector: str, *, default: str | None = None) -> str | None: + def alt( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: """Return the `alt` attribute of the element matching `selector`.""" - return self._get_one("alt", selector, default=default) + return self._get_one("alt", selector, default=default, first=first) def alts(self, selector: str) -> list[str]: """Return the `alt` attribute of the elements matching `selector`.""" return self._get_all("alt", selector) @overload - def title(self, selector: str, *, default: str) -> str: ... + def title( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def title(self, selector: str, *, default: str | None = None) -> str | None: ... + def title( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... - def title(self, selector: str, *, default: str | None = None) -> str | None: + def title( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: """Return the `title` attribute of the element matching `selector`.""" - return self._get_one("title", selector, default=default) + return self._get_one("title", selector, default=default, first=first) def titles(self, selector: str) -> list[str]: """Return the `title` attribute of the elements matching `selector`.""" return self._get_all("title", selector) @overload - def value(self, selector: str, *, default: str) -> str: ... + def value( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def value(self, selector: str, *, default: str | None = None) -> str | None: ... + def value( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... - def value(self, selector: str, *, default: str | None = None) -> str | None: + def value( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: """Return the `value` attribute of the element matching `selector`.""" - return self._get_one("value", selector, default=default) + return self._get_one("value", selector, default=default, first=first) def values(self, selector: str) -> list[str]: """Return the `value` attribute of the elements matching `selector`.""" return self._get_all("value", selector) @overload - def attr(self, attr: str, selector: str, *, default: str) -> str: ... + def attr( + self, + attr: str, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload def attr( @@ -131,6 +232,7 @@ def attr( selector: str, *, default: str | None = None, + first: bool = False, ) -> str | None: ... def attr( @@ -139,56 +241,106 @@ def attr( selector: str, *, default: str | None = None, + first: bool = False, ) -> str | None: """Return the given `attr` attribute of the element matching `selector`.""" - return self._get_one(attr, selector, default=default) + return self._get_one(attr, selector, default=default, first=first) def attrs(self, attr: str, selector: str) -> list[str]: """Return the given `attr` attribute of the elements matching `selector`.""" return self._get_all(attr, selector) @overload - def id(self, selector: str, *, default: str) -> str: ... + def id( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def id(self, selector: str, *, default: str | None = None) -> str | None: ... + def id( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... - def id(self, selector: str, *, default: str | None = None) -> str | None: + def id( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: """Return the `id` attribute of the element matching `selector`.""" - return self._get_one("id", selector, default=default) + return self._get_one("id", selector, default=default, first=first) def ids(self, selector: str) -> list[str]: """Return the `id` attribute of the elements matching `selector`.""" return self._get_all("id", selector) @overload - def content(self, selector: str, *, default: str) -> str: ... + def content( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def content(self, selector: str, *, default: str | None = None) -> str | None: ... + def content( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... - def content(self, selector: str, *, default: str | None = None) -> str | None: + def content( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: """Return the `content` attribute of the element matching `selector`.""" - return self._get_one("content", selector, default=default) + return self._get_one("content", selector, default=default, first=first) def contents(self, selector: str) -> list[str]: """Return the `content` attribute of the elements matching `selector`.""" return self._get_all("content", selector) @overload - def text(self, selector: str, *, default: str) -> str: ... + def text( + self, + selector: str, + *, + default: str, + first: bool = False, + ) -> str: ... @overload - def text(self, selector: str, *, default: str | None = None) -> str | None: ... + def text( + self, + selector: str, + *, + default: str | None = None, + first: bool = False, + ) -> str | None: ... def text( self, selector: str, *, default: str | None = None, + first: bool = False, ) -> str | None: """Return the text contained by the element matching `selector`.""" - return self._get_one("text", selector, default=default) + return self._get_one("text", selector, default=default, first=first) def texts(self, selector: str) -> list[str]: """Return a list of the text contained by the elements matching `selector`.""" @@ -210,6 +362,7 @@ def _get_one( selector: str, *, default: str, + first: bool = False, ) -> str: ... @overload @@ -219,6 +372,7 @@ def _get_one( selector: str, *, default: str | None = ..., + first: bool = False, ) -> str | None: ... def _get_one( @@ -227,8 +381,9 @@ def _get_one( selector: str, *, default: str | None = None, + first: bool = False, ) -> str | None: - if (el := self._select_one(selector)) is None: + if (el := self._select_one(selector, first=first)) is None: return default if (value := el.text_content() if attr == "text" else el.get(attr)) is None: return default @@ -241,12 +396,14 @@ def _get_all(self, attr: str, selector: str) -> list[str]: if (value := el.text_content() if attr == "text" else el.get(attr)) ] - def _select_one(self, selector: str) -> HtmlElement | None: + def _select_one(self, selector: str, *, first: bool = False) -> HtmlElement | None: match self.root.cssselect(selector): case []: return None case [element]: return element + case [element, *_] if first: + return element case elements: msg = f"Selector matched {len(elements)} elements: {selector}" raise MultipleElementsReturned(msg) diff --git a/src/comics/comics/anleggsplassen.py b/src/comics/comics/anleggsplassen.py index c5b6da16..393d87fa 100644 --- a/src/comics/comics/anleggsplassen.py +++ b/src/comics/comics/anleggsplassen.py @@ -36,12 +36,11 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: # The comic image has no title, so select it by its container. # The page offers several widths, the widest one first. - urls = article_page.attrs( - "srcset", ".bodytext figure.column picture source" + url = article_page.attr( + "srcset", ".bodytext figure.column picture source", first=True ) - if not urls: + if url is None: continue - url = urls[0] return CrawlerImage(url, title, text) return None diff --git a/src/comics/comics/cyanideandhappiness.py b/src/comics/comics/cyanideandhappiness.py index 07649b02..fbb0c2ba 100644 --- a/src/comics/comics/cyanideandhappiness.py +++ b/src/comics/comics/cyanideandhappiness.py @@ -22,8 +22,8 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: for entry in feed.for_date(pub_date): page = self.parse_page(entry.link) # The comic is the first image on the image server - urls = page.srcs('img[src*="static.explosm.net"]') - if not urls: + url = page.src('img[src*="static.explosm.net"]', first=True) + if url is None: continue - return CrawlerImage(urls[0], entry.title) + return CrawlerImage(url, entry.title) return None diff --git a/src/comics/comics/gucomics.py b/src/comics/comics/gucomics.py index 21231a7f..f1c2b78b 100644 --- a/src/comics/comics/gucomics.py +++ b/src/comics/comics/gucomics.py @@ -23,11 +23,11 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: page_url = f"http://www.gucomics.com/{pub_date:%Y%m%d}" page = self.parse_page(page_url) - title = page.texts("b")[0] + title = page.text("b", default="", first=True) title = title.replace('"', "") title = title.strip() - text = page.texts(".main")[0] + text = page.text(".main", default="", first=True) # If there is a "---", the text after is not about the comic text = text[: text.find("---")] diff --git a/src/comics/comics/joyoftech.py b/src/comics/comics/joyoftech.py index a870fdd5..ced2f79d 100644 --- a/src/comics/comics/joyoftech.py +++ b/src/comics/comics/joyoftech.py @@ -30,10 +30,10 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: page = self.parse_page(entry.link) # Some pages also hold a thumbnail beside the comic - urls = page.srcs(f'img[src="/joyoftech/joyimages/{num}.png"]') - if not urls: - urls = page.srcs(f'img[src*="/joyimages/{num}."]') - if not urls: + url = page.src(f'img[src="/joyoftech/joyimages/{num}.png"]', first=True) + if url is None: + url = page.src(f'img[src*="/joyimages/{num}."]', first=True) + if url is None: continue - return CrawlerImage(urls[0], title) + return CrawlerImage(url, title) return None diff --git a/src/comics/comics/spaceavalanche.py b/src/comics/comics/spaceavalanche.py index 7b883a12..471fc853 100644 --- a/src/comics/comics/spaceavalanche.py +++ b/src/comics/comics/spaceavalanche.py @@ -22,10 +22,9 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: for entry in feed.for_date(pub_date): if "COMIC ARCHIVE" not in entry.tags: continue - urls = entry.content0.srcs('img[src*="/wp-content/uploads/"]') - if not urls: + url = entry.content0.src('img[src*="/wp-content/uploads/"]', first=True) + if url is None: continue - url = urls[0] title = entry.title return CrawlerImage(url, title) return None diff --git a/src/comics/comics/thisishistorictimes.py b/src/comics/comics/thisishistorictimes.py index 9dd596e7..f6f77ac8 100644 --- a/src/comics/comics/thisishistorictimes.py +++ b/src/comics/comics/thisishistorictimes.py @@ -21,9 +21,9 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: for entry in feed.for_date(pub_date): page = self.parse_page(entry.link) # The comic is the first image, followed by unrelated ones - urls = page.srcs('img[src*="/wp-content/uploads/"]') - if not urls: + url = page.src('img[src*="/wp-content/uploads/"]', first=True) + if url is None: continue title = entry.title - return CrawlerImage(urls[0], title) + return CrawlerImage(url, title) return None diff --git a/src/comics/comics/wulffmorgenthaler.py b/src/comics/comics/wulffmorgenthaler.py index dd3aa53a..c587c641 100644 --- a/src/comics/comics/wulffmorgenthaler.py +++ b/src/comics/comics/wulffmorgenthaler.py @@ -20,7 +20,7 @@ class Crawler(CrawlerBase): def crawl(self, pub_date: dt.date) -> CrawlerResult: page_url = f"http://wumo.com/wumo/{pub_date:%Y/%m/%d}" page = self.parse_page(page_url) - urls = page.srcs(f'img[src*="/img/wumo/{pub_date:%Y/%m}"]') - if not urls: + url = page.src(f'img[src*="/img/wumo/{pub_date:%Y/%m}"]', first=True) + if url is None: return None - return CrawlerImage(urls[0]) + return CrawlerImage(url) From 19d17a8f55e9f100442fa45a579fad7c34e993f0 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:14:12 +0200 Subject: [PATCH 06/11] feat: Add element()/elements() to LxmlParser Crawlers that needed to pick a container and then read its children had to reach past the parser into page.root and hand-roll the extraction, losing the default handling, the multiple-match guard and the CSS selectors. element() and elements() return parsers scoped to the matching elements, so the same extraction API keeps working one level down. Selectors on a scoped parser match the element itself as well as its descendants, since cssselect scopes them as descendant-or-self. Four crawlers move off page.root. This also fixes a latent bug in the Evil Inc crawler, which checked the result of xpath() against None, while xpath() returns an empty list when nothing matches. Awkward Zombie and Subnormality keep using page.root: they match on element text and sort by a parsed style attribute, which the selector API does not express. --- docs/crawlers.md | 2 ++ src/comics/aggregator/lxmlparser.py | 36 +++++++++++++++++++++++ src/comics/comics/anleggsplassen.py | 2 +- src/comics/comics/evilinc.py | 5 ++-- src/comics/comics/optipess.py | 31 +++++++++---------- src/comics/comics/perrybiblefellowship.py | 5 +--- 6 files changed, 56 insertions(+), 25 deletions(-) diff --git a/docs/crawlers.md b/docs/crawlers.md index b3fba5c9..8b64f016 100644 --- a/docs/crawlers.md +++ b/docs/crawlers.md @@ -202,6 +202,8 @@ find the image URL. - attrs - content - contents + - element + - elements - remove - url diff --git a/src/comics/aggregator/lxmlparser.py b/src/comics/aggregator/lxmlparser.py index d044d49e..5d4cfc0e 100644 --- a/src/comics/aggregator/lxmlparser.py +++ b/src/comics/aggregator/lxmlparser.py @@ -35,6 +35,11 @@ class LxmlParser: Pass `first=True` to a singular method to take the first match in document order instead of raising, for pages that legitimately match several elements. + + To extract several values from the same part of a document, use + [`element()`][comics.aggregator.lxmlparser.LxmlParser.element] or + [`elements()`][comics.aggregator.lxmlparser.LxmlParser.elements] to scope + a parser to it. """ _retrieved_url: str | None @@ -346,6 +351,31 @@ def texts(self, selector: str) -> list[str]: """Return a list of the text contained by the elements matching `selector`.""" return self._get_all("text", selector) + def element(self, selector: str, *, first: bool = False) -> LxmlParser | None: + """Return a parser scoped to the element matching `selector`. + + Selectors used on the returned parser match the element itself as + well as its descendants, so a scoped parser can both read the + element's own attributes and dig further into it: + + ```python + for row in page.elements("tr"): + if row.text("td.date") != date_string: + continue + title = row.text("td.title a") + ``` + + Returns `None` if the selector doesn't match any element. + """ + element = self._select_one(selector, first=first) + if element is None: + return None + return self._scoped(element) + + def elements(self, selector: str) -> list[LxmlParser]: + """Return a parser scoped to each of the elements matching `selector`.""" + return [self._scoped(element) for element in self._select_all(selector)] + def remove(self, selector: str) -> None: """Remove the elements matching `selector` from the parsed document.""" for element in self.root.cssselect(selector): @@ -396,6 +426,12 @@ def _get_all(self, attr: str, selector: str) -> list[str]: if (value := el.text_content() if attr == "text" else el.get(attr)) ] + def _scoped(self, element: HtmlElement) -> LxmlParser: + parser = LxmlParser.__new__(LxmlParser) + parser._retrieved_url = self._retrieved_url + parser.root = element + return parser + def _select_one(self, selector: str, *, first: bool = False) -> HtmlElement | None: match self.root.cssselect(selector): case []: diff --git a/src/comics/comics/anleggsplassen.py b/src/comics/comics/anleggsplassen.py index 393d87fa..97b5398a 100644 --- a/src/comics/comics/anleggsplassen.py +++ b/src/comics/comics/anleggsplassen.py @@ -18,7 +18,7 @@ class Crawler(CrawlerBase): def crawl(self, pub_date: dt.date) -> CrawlerResult: page = self.parse_page("https://www.at.no/emne/tegneserie") - articles = page.root.xpath('.//article[@data-section="tegneserie"]/div/a/@href') + articles = page.hrefs('article[data-section="tegneserie"] > div > a') for article in articles: article_page = self.parse_page(article) title = article_page.content('meta[name="title"]') diff --git a/src/comics/comics/evilinc.py b/src/comics/comics/evilinc.py index bbeb1c0e..0aa15f59 100644 --- a/src/comics/comics/evilinc.py +++ b/src/comics/comics/evilinc.py @@ -22,9 +22,8 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: for entry in feed.for_date(pub_date): title = entry.title page = self.parse_page(entry.link) - img = page.root.xpath('//div[@id="unspliced-comic"]/picture/img') - if img is None: + url = page.src("div#unspliced-comic > picture > img", first=True) + if url is None: continue - url = img[0].attrib["src"] return CrawlerImage(url, title) return None diff --git a/src/comics/comics/optipess.py b/src/comics/comics/optipess.py index ca390a0e..503f59f2 100644 --- a/src/comics/comics/optipess.py +++ b/src/comics/comics/optipess.py @@ -23,24 +23,21 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: ) date_string = pub_date.strftime("%b %-d") - post_link = archive_page.root.xpath( - '//td[(@class="archive-date") and ' - f'(.="{date_string}")]/../td[@class="archive-title"]/a' - ) + for row in archive_page.elements("tr"): + if row.text("td.archive-date") != date_string: + continue - if not post_link: - return None - else: - post_link = post_link[0] + title = row.text("td.archive-title a") + post_url = row.href("td.archive-title a") + if post_url is None: + return None - title = post_link.text - # Fetch the actual post - page = self.parse_page(post_link.get("href")) - img = page.root.xpath('//div[@id="comic"]/img') - if not img: - img = page.root.xpath('//div[@id="comic"]/a/img') + # Fetch the actual post + page = self.parse_page(post_url) + # The image is sometimes wrapped in a link + url = page.src("div#comic img", first=True) + text = page.title("div#comic img", first=True) - url = img[0].get("src") - text = img[0].get("title") + return CrawlerImage(url, title, text) - return CrawlerImage(url, title, text) + return None diff --git a/src/comics/comics/perrybiblefellowship.py b/src/comics/comics/perrybiblefellowship.py index a8ad3213..7962db36 100644 --- a/src/comics/comics/perrybiblefellowship.py +++ b/src/comics/comics/perrybiblefellowship.py @@ -20,9 +20,6 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: feed = self.parse_feed("https://pbfcomics.com/feed/") for entry in feed.for_date(pub_date): page = self.parse_page(entry.link) - urls = [ - img.attrib["data-src"] - for img in page.root.findall('.//div[@id="comic"]/img') - ] + urls = page.attrs("data-src", "div#comic > img") return [CrawlerImage(url, entry.title) for url in urls] return None From ecd84509d90b255e93420310ad20d8df1a7c7d76 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:16:02 +0200 Subject: [PATCH 07/11] feat: Declare the Entry fields crawlers actually use link and title reach crawlers through __getattr__, so they typed as Any and nothing checked their use in the 41 feed crawlers. Declaring them alongside the existing summary and content0 annotations types them without changing lookup, as a bare annotation creates no class attribute. --- docs/crawlers.md | 2 ++ src/comics/aggregator/feedparser.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/docs/crawlers.md b/docs/crawlers.md index 8b64f016..72c839f8 100644 --- a/docs/crawlers.md +++ b/docs/crawlers.md @@ -301,6 +301,8 @@ for entry in feed.for_date(pub_date): options: heading_level: 4 members: + - title + - link - summary - content0 - html diff --git a/src/comics/aggregator/feedparser.py b/src/comics/aggregator/feedparser.py index 29a305f0..f28ffd3c 100644 --- a/src/comics/aggregator/feedparser.py +++ b/src/comics/aggregator/feedparser.py @@ -56,6 +56,21 @@ class Entry: [`content0`][comics.aggregator.feedparser.Entry.content0]. """ + title: str + """The entry's title. + + Commonly passed straight on as the + [`CrawlerImage`][comics.aggregator.crawler.CrawlerImage] title. + """ + + link: str + """The URL of the entry, e.g. the comic's page for this release. + + Typically fed to + [`Crawler.parse_page()`][comics.aggregator.crawler.CrawlerBase.parse_page] + when the feed itself doesn't hold the image. + """ + summary: LxmlParser """The entry's summary, with [`LxmlParser`][comics.aggregator.lxmlparser.LxmlParser] methods available From 52bb9b296c88d4eed82b33372955261b9356ae48 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:16:58 +0200 Subject: [PATCH 08/11] refactor: Parse the OOTS feed once per crawl feed.all() rebuilt the whole entry list on each call, and the crawler called it twice to test for and then take the first entry. --- src/comics/comics/oots.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/comics/comics/oots.py b/src/comics/comics/oots.py index d4c61658..518d822b 100644 --- a/src/comics/comics/oots.py +++ b/src/comics/comics/oots.py @@ -18,8 +18,7 @@ class Crawler(CrawlerBase): def crawl(self, pub_date: dt.date) -> CrawlerResult: feed = self.parse_feed("https://www.giantitp.com/comics/oots.rss") - if len(feed.all()): - entry = feed.all()[0] + for entry in feed.all(): page = self.parse_page(entry.link) url = page.src('img[src*="/comics/oots/"]') title = entry.title From 05a28d0aef5d27ab1ba64e767c96e5b1d6bcde1e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:17:19 +0200 Subject: [PATCH 09/11] refactor: Drop the no-op history_length_days = 0 A zero-day history resolves to the same history_start as leaving both history attributes out, which already means only today can be crawled. --- src/comics/comics/devilbear.py | 1 - src/comics/comics/questionablecontent.py | 1 - 2 files changed, 2 deletions(-) diff --git a/src/comics/comics/devilbear.py b/src/comics/comics/devilbear.py index fbfa4d89..473f23ae 100644 --- a/src/comics/comics/devilbear.py +++ b/src/comics/comics/devilbear.py @@ -13,7 +13,6 @@ class Metadata(MetadataBase): class Crawler(CrawlerBase): - history_length_days = 0 schedule = "Tu,We,Th,Fr" time_zone = "America/New_York" diff --git a/src/comics/comics/questionablecontent.py b/src/comics/comics/questionablecontent.py index de23fb97..8685343f 100644 --- a/src/comics/comics/questionablecontent.py +++ b/src/comics/comics/questionablecontent.py @@ -14,7 +14,6 @@ class Metadata(MetadataBase): class Crawler(CrawlerBase): - history_length_days = 0 schedule = "Mo,Tu,We,Th,Su" time_zone = "America/New_York" From 04758390cf8ea4dc7ecf7e57c205fc715b1ea327 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:36:53 +0200 Subject: [PATCH 10/11] refactor: Report a missing image as ImageURLNotFound once the entry is known These three crawlers already establish that the entry is the comic, by its link, its tags or its publishing date. Skipping to the next entry when the image selector then finds nothing reported the crawl as 'no release found' at info level, which is the answer for a day the comic did not publish on, not for a page whose markup moved. Letting the empty URL reach CrawlerImage raises ImageURLNotFound instead, naming the comic and the date at error level. The first of the two guards in the Joy of Tech crawler is the fallback between its two selectors, and stays. --- src/comics/comics/anleggsplassen.py | 2 -- src/comics/comics/joyoftech.py | 2 -- src/comics/comics/spaceavalanche.py | 2 -- 3 files changed, 6 deletions(-) diff --git a/src/comics/comics/anleggsplassen.py b/src/comics/comics/anleggsplassen.py index 97b5398a..f24fe332 100644 --- a/src/comics/comics/anleggsplassen.py +++ b/src/comics/comics/anleggsplassen.py @@ -39,8 +39,6 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: url = article_page.attr( "srcset", ".bodytext figure.column picture source", first=True ) - if url is None: - continue return CrawlerImage(url, title, text) return None diff --git a/src/comics/comics/joyoftech.py b/src/comics/comics/joyoftech.py index ced2f79d..a5f532ac 100644 --- a/src/comics/comics/joyoftech.py +++ b/src/comics/comics/joyoftech.py @@ -33,7 +33,5 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: url = page.src(f'img[src="/joyoftech/joyimages/{num}.png"]', first=True) if url is None: url = page.src(f'img[src*="/joyimages/{num}."]', first=True) - if url is None: - continue return CrawlerImage(url, title) return None diff --git a/src/comics/comics/spaceavalanche.py b/src/comics/comics/spaceavalanche.py index 471fc853..bbd57c02 100644 --- a/src/comics/comics/spaceavalanche.py +++ b/src/comics/comics/spaceavalanche.py @@ -23,8 +23,6 @@ def crawl(self, pub_date: dt.date) -> CrawlerResult: if "COMIC ARCHIVE" not in entry.tags: continue url = entry.content0.src('img[src*="/wp-content/uploads/"]', first=True) - if url is None: - continue title = entry.title return CrawlerImage(url, title) return None From e1c8f66ff9c41dee7aaa37487fe0062607afae7b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 31 Aug 2026 23:37:47 +0200 Subject: [PATCH 11/11] fix: Raise on HTTP errors when parsing a page LxmlParser fetched pages without checking the status, so an error page was parsed as if it were the comic's page. Finding no image in it, a crawler reported 'no release found', which is what a day the comic did not publish on looks like, and a site that moved or started refusing us stayed invisible. httpx.HTTPStatusError is an httpx.HTTPError, which get_release() already wraps into CrawlerHTTPError, so a gone page now reports as a transient failure while a page we did fetch but could not read reports as a broken crawler. --- src/comics/aggregator/lxmlparser.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/comics/aggregator/lxmlparser.py b/src/comics/aggregator/lxmlparser.py index 5d4cfc0e..077530a6 100644 --- a/src/comics/aggregator/lxmlparser.py +++ b/src/comics/aggregator/lxmlparser.py @@ -453,6 +453,7 @@ def _parse_url( headers: dict[str, str] | None = None, ) -> HtmlElement: response = httpx.get(url, headers=headers, follow_redirects=True) + response.raise_for_status() self._retrieved_url = str(response.url) content = response.content.replace(b"\x00", b"") root = self._parse_string(content)