Skip to content

fix(integrations): error paths answer their own status, without the exception text (CRM-874) - #86

Merged
daniloleonecarneiro merged 2 commits into
developfrom
fix/CRM-874-request-indefinido-rotas-integracao
Nov 6, 2026
Merged

daniloleonecarneiro merged 2 commits into
developfrom
fix/CRM-874-request-indefinido-rotas-integracao

Conversation

@daniloleonecarneiro

@daniloleonecarneiro daniloleonecarneiro commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Nas rotas de integração, os blocos de erro de discover, config, PUT e DELETE chamavam error_response(request=request, ...) em funções sem request no escopo. O NameError tomava 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=request removido nas 54 chamadas (o parâmetro é opcional). Onde a função recebe a Request, ficou como estava.
  • Mensagem fixa nas 44 respostas que montavam f"...: {str(e)}". O detalhe continua no logger.error.

Também corrigido, porque o teste pegou o mesmo 500 cru em blocos de erro dessas rotas:

  • Asana, DELETE: passava request=Request, a classe.
  • Google Calendar, PUT, /availability e /events: o request dessas funções é o corpo pydantic, que o error_response não consegue ler.
  • GitHub, GET config: a rota embrulhava a resposta do helper num segundo success_response. No sucesso, data trazia o JSONResponse serializado (status_code, body em string) em vez da config. No erro, sem o NameError, a rota passaria a responder 200 com success: true e o erro dentro. Agora devolve a resposta do helper, como os outros provedores.
  • Canva e GET de /integrations: os mesmos blocos montavam a mensagem com str(e) (4 no Canva, 1 na listagem de configs). Esses arquivos não tinham request indefinido, 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: nenhum undefined name 'request'; os outros 38 avisos são os mesmos da develop
  • tests/unit/test_integration_route_errors.py (novo), test_integrations_rbac_gates.py e test_oauth_callback_contract.py: 180 passed
  • O teste novo contra o src da 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)
  • Rotas montadas com TestClient na imagem do processor, antes e depois:
develop branch
Monday PUT sem integração 500 cru (NameError) 404 "integration not found"
Monday DELETE com o service falhando 500 cru 500 "Failed to disconnect", sem o texto da exceção
Google Calendar PUT / availability / events 500 cru 500 com mensagem fixa

…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.
@sourcery-ai

sourcery-ai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

Fixes integration error paths that previously crashed with NameError or misused request objects, ensuring intended status codes and error envelopes are returned without exposing exception text; GitHub helper failures are now propagated correctly, with broad regression coverage added.

Sequence diagram for integration error response handling

sequenceDiagram
    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
Loading

Sequence diagram for GitHub configuration error propagation

sequenceDiagram
    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
Loading

Flow diagram for corrected integration error paths

flowchart 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]
Loading

File-Level Changes

Change Details Files
Removed invalid request arguments from integration error responses and standardized client-facing failure messages.
  • Removed 54 request=request references where handlers lack a compatible request object, while preserving valid usages.
  • Replaced exception-interpolated messages with fixed messages while retaining exception details in logging.
  • Corrected Asana DELETE to pass the actual request and Google Calendar body-based routes to omit the Pydantic body.
  • Applied the same generic error-message behavior across discovery, configuration, update, save, and disconnect paths.
src/api/asana_routes.py
src/api/atlassian_routes.py
src/api/github_routes.py
src/api/google_calendar_routes.py
src/api/google_sheets_routes.py
src/api/hubspot_routes.py
src/api/linear_routes.py
src/api/mcp_integration_base.py
src/api/monday_routes.py
src/api/notion_routes.py
src/api/paypal_routes.py
src/api/supabase_routes.py
Preserved downstream error responses instead of wrapping them as successful GitHub configuration results.
  • Returned the helper response directly when its status code indicates failure.
  • Left the successful configuration path unchanged.
src/api/github_routes.py
Added integration-route regression coverage for status correctness, response envelopes, and exception-detail redaction.
  • Parameterized failure tests across MCP and Google providers for discovery, reads, writes, disconnects, and listing routes.
  • Covered Google Calendar routes whose request parameter is a Pydantic body.
  • Asserted expected status/error codes, success: false, and absence of the upstream exception marker.
tests/unit/test_integration_route_errors.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

…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.
@daniloleonecarneiro
daniloleonecarneiro merged commit d73d883 into develop Nov 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant