Skip to content

Commit 44025bf

Browse files
committed
Fix follower error handling when leader returns invalid compatibility response
1 parent 0f17364 commit 44025bf

File tree

3 files changed

+34
-20
lines changed

3 files changed

+34
-20
lines changed

src/karapace/api/forward_client.py

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from karapace.version import __version__
1010
from pydantic import BaseModel
1111
from typing import overload, TypeVar, Union
12-
12+
from karapace.api.routers.requests import ErrorResponse
1313
import aiohttp
1414
import async_timeout
1515
import logging
@@ -46,7 +46,7 @@ async def _forward_request_remote(
4646
*,
4747
request: Request,
4848
primary_url: str,
49-
) -> bytes:
49+
) -> tuple[bytes, int]: # Return both body and status code
5050
LOG.info("Forwarding %s request to remote url: %r since we're not the master", request.method, request.url)
5151
timeout = 60.0
5252
func = getattr(self._forward_client, request.method.lower())
@@ -60,16 +60,11 @@ async def _forward_request_remote(
6060
async with func(
6161
forward_url, headers=request.headers.mutablecopy(), data=body_data, ssl=self._ssl_context
6262
) as response:
63-
if self._acceptable_response_content_type(content_type=response.headers.get("Content-Type")):
64-
return await response.text()
65-
LOG.error("Unknown response for forwarded request: %s", response)
66-
raise HTTPException(
67-
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
68-
detail={
69-
"error_code": status.HTTP_500_INTERNAL_SERVER_ERROR,
70-
"message": "Unknown response for forwarded request.",
71-
},
72-
)
63+
body = await response.text()
64+
# Return body and status regardless of content type for error cases
65+
if not self._acceptable_response_content_type(content_type=response.headers.get("Content-Type")):
66+
LOG.error("Unknown response content type for forwarded request: %s", response.headers.get("Content-Type"))
67+
return body, response.status
7368

7469
@overload
7570
async def forward_request_remote(
@@ -90,13 +85,27 @@ async def forward_request_remote(
9085
) -> SimpleTypeResponse: ...
9186

9287
async def forward_request_remote(
93-
self,
94-
*,
95-
request: Request,
96-
primary_url: str,
97-
response_type: type[BaseModelResponse] | type[SimpleTypeResponse],
98-
) -> BaseModelResponse | SimpleTypeResponse:
99-
body = await self._forward_request_remote(request=request, primary_url=primary_url)
88+
self,
89+
*,
90+
request: Request,
91+
primary_url: str,
92+
response_type: type[BaseModelResponse] | type[SimpleTypeResponse],
93+
) -> BaseModelResponse | SimpleTypeResponse:
94+
body, http_status = await self._forward_request_remote(request=request, primary_url=primary_url)
95+
96+
# If the leader returned an error status, parse and re-raise it
97+
if http_status >= 400:
98+
try:
99+
error_data = json_decode(body)
100+
except Exception:
101+
error_data = {"error_code": http_status, "message": body}
102+
103+
raise HTTPException(
104+
status_code=http_status, # Use HTTP status, not error_code from body
105+
detail=error_data,
106+
)
107+
108+
# Success case - parse according to expected response type
100109
if response_type is int:
101110
return int(body) # type: ignore[return-value]
102111
if response_type == list[int]:

src/karapace/api/routers/requests.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,7 @@ class SubjectSchemaVersionResponse(BaseModel):
9898
references: list[Any] | None = None
9999
schema_type: SchemaType | None = Field(alias="schemaType", default=None)
100100
compatibility: str | None = None
101+
102+
class ErrorResponse(BaseModel):
103+
error_code: int
104+
message: str

src/karapace/core/auth.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ def __init__(self, *, user_db: dict[str, User] | None = None, permissions: list[
154154
def get_user(self, username: str) -> User | None:
155155
user = self.user_db.get(username)
156156
if not user:
157-
raise ValueError("No user found")
157+
#return None instead of raising ValueError as this error is being handled as AuthenticationError
158+
return None
158159
return user
159160

160161
def _check_resources(self, resources: list[str], aclentry: ACLEntry) -> bool:

0 commit comments

Comments
 (0)