From 4d3aa35903fa2df061f89b6bfab7c185caf114c8 Mon Sep 17 00:00:00 2001 From: Benjamin Schmitz Date: Tue, 28 Jul 2026 20:24:34 +0200 Subject: [PATCH] fix: surface ticket creation failures to the client Ticket creation errors were caught and swallowed on the server, so the client received 201 Created and rendered "submitted successfully" even when no ticket was produced. The DB now stays uncommitted until the ticket succeeds, so a failed ticket rolls back the request row and returns 502, letting the existing client error UI kick in. Co-Authored-By: Claude Opus 4.7 (1M context) --- server/request_server/api/routes/_ticket.py | 52 +++++++++++++++++++ .../api/routes/artemis_developer_requests.py | 35 +++++-------- .../api/routes/support_requests.py | 29 +++++------ .../api/routes/tum_guest_requests.py | 29 +++++------ .../api/routes/vm_access_requests.py | 29 +++++------ .../request_server/api/routes/vm_requests.py | 29 +++++------ 6 files changed, 115 insertions(+), 88 deletions(-) create mode 100644 server/request_server/api/routes/_ticket.py diff --git a/server/request_server/api/routes/_ticket.py b/server/request_server/api/routes/_ticket.py new file mode 100644 index 0000000..7370968 --- /dev/null +++ b/server/request_server/api/routes/_ticket.py @@ -0,0 +1,52 @@ +"""Shared helpers for handling ticket creation across request routes.""" + +from __future__ import annotations + +import logging +from collections.abc import Awaitable +from typing import Any + +from fastapi import HTTPException, status + +from request_server.core.config import settings + +_TICKET_FAILURE_DETAIL = ( + "The request could not be forwarded to our ticket system. Please try again later." +) + + +async def raise_on_ticket_failure( + ticket_task: Awaitable[str | None], + *, + entity_id: Any, + entity_label: str, + logger: logging.Logger, +) -> str | None: + """Await a ticket-creation coroutine and raise HTTPException on failure. + + Returns the ticket key on success, or None when the configured ticket + system deliberately produces no ticket (e.g. NoOp). Callers should defer + committing the request row until after this returns so an exception + triggers a rollback of the pending INSERT. + """ + try: + ticket_key = await ticket_task + except Exception as exc: + logger.exception("Error creating ticket for %s %s", entity_label, entity_id) + raise HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=_TICKET_FAILURE_DETAIL, + ) from exc + + if ticket_key: + logger.info("Created ticket %s for %s %s", ticket_key, entity_label, entity_id) + return ticket_key + + if settings.ticket_system_enabled: + logger.error("Ticket service returned no key for %s %s", entity_label, entity_id) + raise HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=_TICKET_FAILURE_DETAIL, + ) + + return None diff --git a/server/request_server/api/routes/artemis_developer_requests.py b/server/request_server/api/routes/artemis_developer_requests.py index f72b8b1..0b10da9 100644 --- a/server/request_server/api/routes/artemis_developer_requests.py +++ b/server/request_server/api/routes/artemis_developer_requests.py @@ -8,7 +8,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from request_server.core.config import settings +from request_server.api.routes._ticket import raise_on_ticket_failure from request_server.core.security import CurrentUser, get_current_user, get_optional_current_user from request_server.db.session import get_db from request_server.models.artemis_developer_request import ( @@ -82,34 +82,25 @@ async def create_artemis_developer_request( ) db.add(artemis_request) - await db.commit() + await db.flush() await db.refresh(artemis_request) - # Create ticket in the configured ticket system - try: - ticket_key = await handle_artemis_ticket_creation( + ticket_key = await raise_on_ticket_failure( + handle_artemis_ticket_creation( get_ticket_service(), artemis_request, is_authenticated=is_authenticated, requester_username=current_user.username if current_user else None, - ) - if ticket_key: - artemis_request.jira_ticket_key = ticket_key - await db.commit() - await db.refresh(artemis_request) - logger.info( - f"Created ticket {ticket_key} for Artemis developer request {artemis_request.id}" - ) - elif settings.ticket_system != "debug": - logger.warning( - f"Failed to create ticket for Artemis developer request {artemis_request.id}" - ) - except Exception as e: - logger.error( - f"Error creating ticket for Artemis developer request {artemis_request.id}: {e}" - ) - # Don't fail the request if ticket creation fails + ), + entity_id=artemis_request.id, + entity_label="Artemis developer request", + logger=logger, + ) + if ticket_key: + artemis_request.jira_ticket_key = ticket_key + await db.commit() + await db.refresh(artemis_request) return artemis_request diff --git a/server/request_server/api/routes/support_requests.py b/server/request_server/api/routes/support_requests.py index f0f9f54..08699ad 100644 --- a/server/request_server/api/routes/support_requests.py +++ b/server/request_server/api/routes/support_requests.py @@ -8,7 +8,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from request_server.core.config import settings +from request_server.api.routes._ticket import raise_on_ticket_failure from request_server.core.security import CurrentUser, get_current_user, get_optional_current_user from request_server.db.session import get_db from request_server.models.support_request import ( @@ -68,28 +68,25 @@ async def create_support_request( ) db.add(support_request) - await db.commit() + await db.flush() await db.refresh(support_request) - # Create ticket in the configured ticket system - try: - ticket_key = await handle_support_ticket_creation( + ticket_key = await raise_on_ticket_failure( + handle_support_ticket_creation( get_ticket_service(), support_request, is_authenticated=is_authenticated, requester_username=current_user.username if current_user else None, - ) - if ticket_key: - support_request.jira_ticket_key = ticket_key - await db.commit() - await db.refresh(support_request) - logger.info(f"Created ticket {ticket_key} for support request {support_request.id}") - elif settings.ticket_system != "debug": - logger.warning(f"Failed to create ticket for support request {support_request.id}") - except Exception as e: - logger.error(f"Error creating ticket for support request {support_request.id}: {e}") - # Don't fail the request if ticket creation fails + ), + entity_id=support_request.id, + entity_label="support request", + logger=logger, + ) + if ticket_key: + support_request.jira_ticket_key = ticket_key + await db.commit() + await db.refresh(support_request) return support_request diff --git a/server/request_server/api/routes/tum_guest_requests.py b/server/request_server/api/routes/tum_guest_requests.py index 3c692c9..db6de4e 100644 --- a/server/request_server/api/routes/tum_guest_requests.py +++ b/server/request_server/api/routes/tum_guest_requests.py @@ -8,7 +8,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from request_server.core.config import settings +from request_server.api.routes._ticket import raise_on_ticket_failure from request_server.core.security import CurrentUser, get_current_user, get_optional_current_user from request_server.db.session import get_db from request_server.models.tum_guest_request import Gender as GenderModel @@ -109,28 +109,25 @@ async def create_tum_guest_request( ) db.add(guest_request) - await db.commit() + await db.flush() await db.refresh(guest_request) - # Create ticket in the configured ticket system - try: - ticket_key = await handle_tum_guest_ticket_creation( + ticket_key = await raise_on_ticket_failure( + handle_tum_guest_ticket_creation( get_ticket_service(), guest_request, is_authenticated=is_authenticated, requester_username=current_user.username if current_user else None, - ) - if ticket_key: - guest_request.jira_ticket_key = ticket_key - await db.commit() - await db.refresh(guest_request) - logger.info(f"Created ticket {ticket_key} for TUM guest request {guest_request.id}") - elif settings.ticket_system != "debug": - logger.warning(f"Failed to create ticket for TUM guest request {guest_request.id}") - except Exception as e: - logger.error(f"Error creating ticket for TUM guest request {guest_request.id}: {e}") - # Don't fail the request if ticket creation fails + ), + entity_id=guest_request.id, + entity_label="TUM guest request", + logger=logger, + ) + if ticket_key: + guest_request.jira_ticket_key = ticket_key + await db.commit() + await db.refresh(guest_request) return guest_request diff --git a/server/request_server/api/routes/vm_access_requests.py b/server/request_server/api/routes/vm_access_requests.py index 9f9ed62..7ba58db 100644 --- a/server/request_server/api/routes/vm_access_requests.py +++ b/server/request_server/api/routes/vm_access_requests.py @@ -8,8 +8,8 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from request_server.api.routes._ticket import raise_on_ticket_failure from request_server.api.routes.ssh_keys import parse_ssh_key -from request_server.core.config import settings from request_server.core.security import CurrentUser, get_current_user from request_server.db.session import get_db from request_server.models.ssh_key import SSHKey @@ -117,25 +117,20 @@ async def create_vm_access_request( ) db.add(access_request) - await db.commit() + await db.flush() await db.refresh(access_request) - # Create ticket in the configured ticket system - try: - ticket_key = await handle_vm_access_ticket_creation( - get_ticket_service(), access_request, ssh_public_key - ) - if ticket_key: - access_request.jira_ticket_key = ticket_key - await db.commit() - await db.refresh(access_request) - logger.info(f"Created ticket {ticket_key} for VM access request {access_request.id}") - elif settings.ticket_system != "debug": - logger.warning(f"Failed to create ticket for VM access request {access_request.id}") - except Exception as e: - logger.error(f"Error creating ticket for VM access request {access_request.id}: {e}") - # Don't fail the request if ticket creation fails + ticket_key = await raise_on_ticket_failure( + handle_vm_access_ticket_creation(get_ticket_service(), access_request, ssh_public_key), + entity_id=access_request.id, + entity_label="VM access request", + logger=logger, + ) + if ticket_key: + access_request.jira_ticket_key = ticket_key + await db.commit() + await db.refresh(access_request) return access_request diff --git a/server/request_server/api/routes/vm_requests.py b/server/request_server/api/routes/vm_requests.py index afebbdb..d2084b8 100644 --- a/server/request_server/api/routes/vm_requests.py +++ b/server/request_server/api/routes/vm_requests.py @@ -6,8 +6,8 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from request_server.api.routes._ticket import raise_on_ticket_failure from request_server.api.routes.ssh_keys import parse_ssh_key -from request_server.core.config import settings from request_server.core.security import CurrentUser, get_current_user from request_server.db.session import get_db from request_server.models.ssh_key import SSHKey @@ -134,25 +134,20 @@ async def create_vm_request( ) db.add(vm_request) - await db.commit() + await db.flush() await db.refresh(vm_request) - # Create ticket in the configured ticket system - try: - ticket_key = await handle_vm_ticket_creation( - get_ticket_service(), vm_request, ssh_public_key - ) - if ticket_key: - vm_request.jira_ticket_key = ticket_key - await db.commit() - await db.refresh(vm_request) - logger.info(f"Created ticket {ticket_key} for VM request {vm_request.id}") - elif settings.ticket_system != "debug": - logger.warning(f"Failed to create ticket for VM request {vm_request.id}") - except Exception as e: - logger.error(f"Error creating ticket for VM request {vm_request.id}: {e}") - # Don't fail the request if ticket creation fails + ticket_key = await raise_on_ticket_failure( + handle_vm_ticket_creation(get_ticket_service(), vm_request, ssh_public_key), + entity_id=vm_request.id, + entity_label="VM request", + logger=logger, + ) + if ticket_key: + vm_request.jira_ticket_key = ticket_key + await db.commit() + await db.refresh(vm_request) return vm_request