Repository navigation
fix(integrations): error paths answer their own status, without the exception text (CRM-874) - #86
Merged
daniloleonecarneiro merged 2 commits intoNov 6, 2026
Conversation
…xception text (CRM-874) The discover, config, PUT and DELETE error blocks passed request=request where no request was in scope, so the NameError replaced the intended answer (a 404 for a missing integration became a bare 500). Asana's DELETE passed the Request class, and Google Calendar's PUT, availability and events passed their pydantic body, which crashed the same way. GitHub's GET config now returns the helper's error instead of wrapping it in a 200. Responses carry a fixed message; the exception stays in the log.
Reviewer's GuideFixes integration error paths that previously crashed with Sequence diagram for integration error response handlingsequenceDiagram
participant Client
participant Route
participant error_response
participant Logger
Client->>Route: PUT or DELETE integration configuration
Route->>Logger: logger.error(exception)
Route->>error_response: error_response(code, message, status_code)
error_response-->>Route: Error envelope with intended status
Route-->>Client: 404 or 500 response without exception text
Sequence diagram for GitHub configuration error propagationsequenceDiagram
participant Caller
participant load_from_service
participant get_configuration_endpoint
participant error_response
Caller->>load_from_service: load_from_service(ag_id)
load_from_service->>get_configuration_endpoint: get_configuration_endpoint(agent_id, github, db, load_from_service)
get_configuration_endpoint->>error_response: error_response(code, message, status_code)
error_response-->>get_configuration_endpoint: Error response
get_configuration_endpoint-->>load_from_service: configuration
load_from_service->>load_from_service: configuration.status_code
load_from_service-->>Caller: Propagated error response
Flow diagram for corrected integration error pathsflowchart LR
A[Integration route encounters failure] --> B["logger.error(exception)"]
B --> C[error_response without request dependency]
C --> D[Fixed public message]
D --> E[HTTP response with intended status]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
7 tasks done
…st, return GitHub's config unwrapped (CRM-874) Canva's discover, config, PUT and DELETE and the GET of all configs still built their message from str(e). GitHub's GET config wrapped the helper's JSONResponse inside data; it now returns the helper's response like the other providers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Nas rotas de integração, os blocos de erro de discover, config, PUT e DELETE chamavam
error_response(request=request, ...)em funções semrequestno escopo. ONameErrortomava o lugar da resposta pretendida: o PUT sem integração conectada, que devia dar 404, saía como 500 cru ("Internal Server Error", sem o envelope). O pyflakes acusava 54 ocorrências em 12 arquivos.request=requestremovido nas 54 chamadas (o parâmetro é opcional). Onde a função recebe aRequest, ficou como estava.f"...: {str(e)}". O detalhe continua nologger.error.Também corrigido, porque o teste pegou o mesmo 500 cru em blocos de erro dessas rotas:
request=Request, a classe./availabilitye/events: orequestdessas funções é o corpo pydantic, que oerror_responsenão consegue ler.success_response. No sucesso,datatrazia oJSONResponseserializado (status_code,bodyem string) em vez da config. No erro, sem oNameError, a rota passaria a responder 200 comsuccess: truee o erro dentro. Agora devolve a resposta do helper, como os outros provedores./integrations: os mesmos blocos montavam a mensagem comstr(e)(4 no Canva, 1 na listagem de configs). Esses arquivos não tinhamrequestindefinido, por isso não estavam na lista do pyflakes.Fora do escopo: os outros 38 avisos do pyflakes em
src/api(imports sem uso etc.) continuam iguais.Test plan
python -m pyflakes src/api: nenhumundefined name 'request'; os outros 38 avisos são os mesmos da developtests/unit/test_integration_route_errors.py(novo),test_integrations_rbac_gates.pyetest_oauth_callback_contract.py: 180 passedsrcda develop: 52 de 53 vermelhos (1º commit); mutações do 2º commit, cada uma vermelha:str(e)de volta no Canva e no/integrations(5 casos), GitHub com o embrulho de volta (1 caso)TestClientna imagem do processor, antes e depois:NameError)