feat(providers): refactoring providers endpoints#696
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| async def create_provider( | ||
| request: Request, | ||
| body: CreateProvider, | ||
| create_provider_use_case: CreateProviderUseCase = Depends(create_provider_use_case_factory), | ||
| request_context: RequestContext = Depends(get_request_context), | ||
| ) -> CreateProviderResponse: |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, to fix “explicit returns mixed with implicit returns,” every control-flow path in the function should either explicitly return a value or explicitly raise an exception. That means adding a final return (or raise) after conditional logic that might otherwise fall through to the end of the function.
For this specific function, all expected “error” outcomes are already mapped to raise statements in the match result: block, and the happy path has an explicit return. The best way to avoid an implicit return, without changing the intended behavior, is to add a final case _: branch in the match statement that handles any unexpected result value by logging and raising an InternalServerHTTPException. This keeps the external behavior consistent: the endpoint still either returns a CreateProviderResponse or raises an HTTP exception. It also guards against future changes that introduce new result variants without updating this endpoint.
Concretely:
- Edit
api/infrastructure/fastapi/endpoints/admin/providers.py. - Inside
create_provider, after the lastcase InsufficientPermissionError():block (line 102–103), add acase _:block that logs the unexpected result (optionally) and raisesInternalServerHTTPException(). - No new imports are needed, since
InternalServerHTTPExceptionandloggerare already imported/defined.
| @@ -101,6 +101,18 @@ | ||
| raise RouterNotFoundHTTPException(router_id=router_id) | ||
| case InsufficientPermissionError(): | ||
| raise InsufficientPermissionHTTPException() | ||
| case _: | ||
| logger.error( | ||
| "Unexpected result type from create_provider_use_case.execute", | ||
| extra={ | ||
| "user_id": request_context.get().user_id, | ||
| "provider_router_id": body.router, | ||
| "provider_url": body.url, | ||
| "provider_model_name": body.model_name, | ||
| "result_type": type(result).__name__, | ||
| }, | ||
| ) | ||
| raise InternalServerHTTPException() | ||
|
|
||
|
|
||
| @router.delete( |
benjaminpilia
force-pushed
the
refacto_providers_endpoints
branch
from
a258a0d to
22a95b7
Compare
benjaminpilia
force-pushed
the
refacto_providers_endpoints
branch
from
cba79f3 to
48dc445
Compare
|
|
||
| self.handle_delete_entity_dialog_change(is_open=False) | ||
| yield rx.toast.success("Provider deleted successfully", position="bottom-right") | ||
| yield rx.toast.success("provider deleted successfully", position="bottom-right") |
| response.raise_for_status() | ||
|
|
||
| yield rx.toast.success("Provider created successfully", position="bottom-right") | ||
| yield rx.toast.success("provider created successfully", position="bottom-right") |
|
|
||
| self.handle_settings_entity_dialog_change(is_open=False) | ||
| yield rx.toast.success("Provider updated successfully", position="bottom-right") | ||
| yield rx.toast.success("provider updated successfully", position="bottom-right") |
| @router.post( | ||
| path=EndpointRoute.ADMIN_PROVIDERS, | ||
| dependencies=[Security(dependency=get_current_key)], | ||
| status_code=201, |
There was a problem hiding this comment.
Déclarer les erreurs ici pour la génération de la documentation redoc.
…pe before reach provider API
| self.provider_gateway = provider_gateway | ||
| self.user_info_repository = user_info_repository | ||
|
|
||
| async def execute(self, command: CreateProviderCommand) -> CreateProviderUseCaseResult: |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, to fix “explicit returns mixed with implicit (fall through) returns” you add an explicit return at the end of the function so that no control-flow path can implicitly return None. The explicit return should return a value that is consistent with the function’s annotated return type and with how callers are expected to handle results.
For CreateProviderUseCase.execute, the single best fix is to add a final return statement at the end of the method that returns a value of type CreateProviderUseCaseResult. Since all realistic code paths are already covered by prior returns, this final return will be unreachable in normal execution, serving only as a safety net to satisfy static analysis and future-proof the function against accidental fallthrough. A minimal and non-breaking way is to return a default error type that is already part of the union; given the existing union members, a reasonable choice is ProviderNotReachableError(), which is a generic failure indicating that the provider could not be reached or created. Concretely, in api/use_cases/admin/providers/_createproviderusecase.py, after the final match result: block in execute (after line 126), add a new line with an explicit return ProviderNotReachableError(). No new imports or additional definitions are needed, since ProviderNotReachableError is already imported at the top of the file.
| @@ -124,3 +124,5 @@ | ||
| return CreateProviderUseCaseSuccess(provider) | ||
| case error: | ||
| return error | ||
|
|
||
| return ProviderNotReachableError() |
leoguillaume
changed the title
3 participants
No description provided.