Skip to content

Don't raise HTTPStatusError on CORS errors when querying packages in a transaction #225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions micropip/_compat/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@

to_js = compatibility_layer.to_js

HttpStatusError = compatibility_layer.HttpStatusError


__all__ = [
"LOCKFILE_INFO",
"LOCKFILE_PACKAGES",
Expand All @@ -47,5 +44,4 @@
"loadPackage",
"get_dynlibs",
"to_js",
"HttpStatusError",
]
18 changes: 4 additions & 14 deletions micropip/_compat/_compat_in_pyodide.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from pyodide._package_loader import get_dynlibs
from pyodide.ffi import IN_BROWSER, to_js
from pyodide.http import HttpStatusError, pyfetch
from pyodide.http import pyfetch

Check warning on line 10 in micropip/_compat/_compat_in_pyodide.py

View check run for this annotation

Codecov / codecov/patch

micropip/_compat/_compat_in_pyodide.py#L10

Added line #L10 was not covered by tests

from .compatibility_layer import CompatibilityLayer

Expand All @@ -28,14 +28,6 @@


class CompatibilityInPyodide(CompatibilityLayer):
class HttpStatusError(Exception):
status_code: int
message: str

def __init__(self, status_code: int, message: str):
self.status_code = status_code
self.message = message
super().__init__(message)

@staticmethod
async def fetch_bytes(url: str, kwargs: dict[str, str]) -> bytes:
Expand All @@ -51,11 +43,9 @@
async def fetch_string_and_headers(
url: str, kwargs: dict[str, str]
) -> tuple[str, dict[str, str]]:
try:
response = await pyfetch(url, **kwargs)
response.raise_for_status()
except HttpStatusError as e:
raise CompatibilityInPyodide.HttpStatusError(e.status, str(e)) from e

response = await pyfetch(url, **kwargs)
response.raise_for_status()

Check warning on line 48 in micropip/_compat/_compat_in_pyodide.py

View check run for this annotation

Codecov / codecov/patch

micropip/_compat/_compat_in_pyodide.py#L47-L48

Added lines #L47 - L48 were not covered by tests

content = await response.string()
headers: dict[str, str] = response.headers
Expand Down
16 changes: 1 addition & 15 deletions micropip/_compat/_compat_not_in_pyodide.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import re
from pathlib import Path
from typing import IO, TYPE_CHECKING, Any
from urllib.error import HTTPError
from urllib.request import Request, urlopen
from urllib.response import addinfourl

Expand All @@ -17,15 +16,6 @@ class CompatibilityNotInPyodide(CompatibilityLayer):
# TODO: use packaging APIs here instead?
_canonicalize_regex = re.compile(r"[-_.]+")

class HttpStatusError(Exception):
status_code: int
message: str

def __init__(self, status_code: int, message: str):
self.status_code = status_code
self.message = message
super().__init__(message)

class loadedPackages(CompatibilityLayer.loadedPackages):
@staticmethod
def to_py():
Expand All @@ -43,11 +33,7 @@ async def fetch_bytes(url: str, kwargs: dict[str, Any]) -> bytes:
async def fetch_string_and_headers(
url: str, kwargs: dict[str, Any]
) -> tuple[str, dict[str, str]]:
try:
response = CompatibilityNotInPyodide._fetch(url, kwargs=kwargs)
except HTTPError as e:
raise CompatibilityNotInPyodide.HttpStatusError(e.code, str(e)) from e

response = CompatibilityNotInPyodide._fetch(url, kwargs=kwargs)
headers = {k.lower(): v for k, v in response.headers.items()}
return response.read().decode(), headers

Expand Down
8 changes: 0 additions & 8 deletions micropip/_compat/compatibility_layer.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@ class CompatibilityLayer(ABC):
All of the following methods / properties must be implemented for use both inside and outside of pyodide.
"""

class HttpStatusError(ABC, Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove it from here and here too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in abc1437; I didn't realise you were asking to remove HttpStatusErrors completely.

status_code: int
message: str

@abstractmethod
def __init__(self, status_code: int, message: str):
pass

class loadedPackages(ABC):
@staticmethod
@abstractmethod
Expand Down
14 changes: 7 additions & 7 deletions micropip/package_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from typing import Any
from urllib.parse import urljoin, urlparse, urlunparse

from ._compat import HttpStatusError, fetch_string_and_headers
from ._compat import fetch_string_and_headers
from ._utils import is_package_compatible, parse_version
from ._vendored.mousebender.simple import from_project_details_html
from ._vendored.packaging.src.packaging.utils import InvalidWheelFilename
Expand Down Expand Up @@ -313,14 +313,14 @@ async def query_package(
logger.debug("Url has no placeholder, appending package name : %r", url)
try:
metadata, headers = await fetch_string_and_headers(url, _fetch_kwargs)
except HttpStatusError as e:
if e.status_code == 404:
logger.debug("NotFound (404) for %r, trying next index.", url)
continue
except Exception as e:
logger.debug(
"Error fetching %r (%s), trying next index.", url, e.status_code
"Error fetching metadata for the package %r from (%r): %r, trying next index.",
name,
url,
e,
)
raise
continue

content_type = headers.get("content-type", "").lower()
try:
Expand Down
30 changes: 0 additions & 30 deletions tests/test_compat.py

This file was deleted.