From a36e741b9f07350b2ebe306ec69adee2a7d6ea14 Mon Sep 17 00:00:00 2001 From: Taha Yassine Kraiem Date: Fri, 2 Aug 2024 14:48:35 +0100 Subject: [PATCH] refactor(chalice): log 422 errors refactor(chalice): cleaned session-replay deprecated code fix(chalice): fixed sessionId to str wrapper --- api/chalicelib/core/sessions_replay.py | 79 +--------------------- api/or_dependencies.py | 9 +++ api/routers/core_dynamic.py | 20 ------ ee/api/chalicelib/core/sessions_replay.py | 82 ----------------------- ee/api/or_dependencies.py | 8 +++ ee/api/routers/core_dynamic.py | 21 ------ 6 files changed, 18 insertions(+), 201 deletions(-) diff --git a/api/chalicelib/core/sessions_replay.py b/api/chalicelib/core/sessions_replay.py index d31d468628..8011a86432 100644 --- a/api/chalicelib/core/sessions_replay.py +++ b/api/chalicelib/core/sessions_replay.py @@ -1,6 +1,6 @@ import schemas from chalicelib.core import events, metadata, events_mobile, \ - sessions_mobs, issues, resources, assist, sessions_devtool, sessions_notes, canvas, user_testing + sessions_mobs, issues, resources, assist, sessions_devtool, canvas, user_testing from chalicelib.utils import errors_helper from chalicelib.utils import pg_client, helper @@ -18,83 +18,6 @@ def __group_metadata(session, project_metadata): return meta -# for backward compatibility -def get_by_id2_pg(project_id, session_id, context: schemas.CurrentContext, full_data=False, include_fav_viewed=False, - group_metadata=False, live=True): - with pg_client.PostgresClient() as cur: - extra_query = [] - if include_fav_viewed: - extra_query.append("""COALESCE((SELECT TRUE - FROM public.user_favorite_sessions AS fs - WHERE s.session_id = fs.session_id - AND fs.user_id = %(userId)s), FALSE) AS favorite""") - extra_query.append("""COALESCE((SELECT TRUE - FROM public.user_viewed_sessions AS fs - WHERE s.session_id = fs.session_id - AND fs.user_id = %(userId)s), FALSE) AS viewed""") - query = cur.mogrify( - f"""\ - SELECT - s.*, - s.session_id::text AS session_id, - (SELECT project_key FROM public.projects WHERE project_id = %(project_id)s LIMIT 1) AS project_key - {"," if len(extra_query) > 0 else ""}{",".join(extra_query)} - {(",json_build_object(" + ",".join([f"'{m}',p.{m}" for m in metadata.column_names()]) + ") AS project_metadata") if group_metadata else ''} - FROM public.sessions AS s {"INNER JOIN public.projects AS p USING (project_id)" if group_metadata else ""} - WHERE s.project_id = %(project_id)s - AND s.session_id = %(session_id)s;""", - {"project_id": project_id, "session_id": session_id, "userId": context.user_id} - ) - cur.execute(query=query) - - data = cur.fetchone() - if data is not None: - data = helper.dict_to_camel_case(data) - if full_data: - if __is_mobile_session(data["platform"]): - data['events'] = events_mobile.get_by_sessionId(project_id=project_id, session_id=session_id) - for e in data['events']: - if e["type"].endswith("_IOS"): - e["type"] = e["type"][:-len("_IOS")] - elif e["type"].endswith("_MOBILE"): - e["type"] = e["type"][:-len("_MOBILE")] - data['crashes'] = events_mobile.get_crashes_by_session_id(session_id=session_id) - data['userEvents'] = events_mobile.get_customs_by_session_id(project_id=project_id, - session_id=session_id) - data['mobsUrl'] = [] - else: - data['events'] = events.get_by_session_id(project_id=project_id, session_id=session_id, - group_clickrage=True) - all_errors = events.get_errors_by_session_id(session_id=session_id, project_id=project_id) - data['stackEvents'] = [e for e in all_errors if e['source'] != "js_exception"] - # to keep only the first stack - # limit the number of errors to reduce the response-body size - data['errors'] = [errors_helper.format_first_stack_frame(e) for e in all_errors - if e['source'] == "js_exception"][:500] - data['userEvents'] = events.get_customs_by_session_id(project_id=project_id, - session_id=session_id) - data['domURL'] = sessions_mobs.get_urls(session_id=session_id, project_id=project_id, - check_existence=False) - data['mobsUrl'] = sessions_mobs.get_urls_depercated(session_id=session_id, check_existence=False) - data['devtoolsURL'] = sessions_devtool.get_urls(session_id=session_id, project_id=project_id, - check_existence=False) - data['resources'] = resources.get_by_session_id(session_id=session_id, project_id=project_id, - start_ts=data["startTs"], duration=data["duration"]) - - data['notes'] = sessions_notes.get_session_notes(tenant_id=context.tenant_id, project_id=project_id, - session_id=session_id, user_id=context.user_id) - data['metadata'] = __group_metadata(project_metadata=data.pop("projectMetadata"), session=data) - data['issues'] = issues.get_by_session_id(session_id=session_id, project_id=project_id) - data['live'] = live and assist.is_live(project_id=project_id, session_id=session_id, - project_key=data["projectKey"]) - data["inDB"] = True - return data - elif live: - return assist.get_live_session_by_id(project_id=project_id, session_id=session_id) - else: - return None - - def get_pre_replay(project_id, session_id, context: schemas.CurrentContext): return { 'domURL': [sessions_mobs.get_first_url(project_id=project_id, session_id=session_id, check_existence=False)]} diff --git a/api/or_dependencies.py b/api/or_dependencies.py index 6ccfbd3e68..16581a10ad 100644 --- a/api/or_dependencies.py +++ b/api/or_dependencies.py @@ -3,6 +3,7 @@ from typing import Callable from fastapi import Depends, Security +from fastapi.exceptions import RequestValidationError from fastapi.routing import APIRoute from fastapi.security import SecurityScopes from starlette import status @@ -31,6 +32,11 @@ async def custom_route_handler(request: Request) -> Response: logger.debug(f"call processed by: {self.methods} {self.path_format}") try: response: Response = await original_route_handler(request) + except RequestValidationError as exc: + # 422 validation exception + logger.warning(f"422 exception when calling: {request.method} {request.url}") + logger.warning(exc.errors()) + raise exc except HTTPException as e: if e.status_code // 100 == 4: return JSONResponse(content={"errors": e.detail if isinstance(e.detail, list) else [e.detail]}, @@ -41,6 +47,8 @@ async def custom_route_handler(request: Request) -> Response: if isinstance(response, JSONResponse): response: JSONResponse = response body = json.loads(response.body.decode('utf8')) + response.body = response.render(helper.cast_session_id_to_string(body)) + response.headers["Content-Length"] = str(len(response.body)) if response.status_code == 200 \ and body is not None and isinstance(body, dict) \ and body.get("errors") is not None: @@ -48,6 +56,7 @@ async def custom_route_handler(request: Request) -> Response: response.status_code = status.HTTP_404_NOT_FOUND else: response.status_code = status.HTTP_400_BAD_REQUEST + return response return custom_route_handler diff --git a/api/routers/core_dynamic.py b/api/routers/core_dynamic.py index b5c6ef1d94..920c747995 100644 --- a/api/routers/core_dynamic.py +++ b/api/routers/core_dynamic.py @@ -223,26 +223,6 @@ def get_projects(context: schemas.CurrentContext = Depends(OR_context)): return {"data": projects.get_projects(tenant_id=context.tenant_id, gdpr=True, recorded=True)} -# for backward compatibility -@app.get('/{projectId}/sessions/{sessionId}', tags=["sessions", "replay"]) -def get_session(projectId: int, sessionId: Union[int, str], background_tasks: BackgroundTasks, - context: schemas.CurrentContext = Depends(OR_context)): - if not sessionId.isnumeric(): - return {"errors": ["session not found"]} - else: - sessionId = int(sessionId) - data = sessions_replay.get_by_id2_pg(project_id=projectId, session_id=sessionId, full_data=True, - include_fav_viewed=True, group_metadata=True, context=context) - if data is None: - return {"errors": ["session not found"]} - if data.get("inDB"): - background_tasks.add_task(sessions_viewed.view_session, project_id=projectId, user_id=context.user_id, - session_id=sessionId) - return { - 'data': data - } - - @app.post('/{projectId}/sessions/search', tags=["sessions"]) def sessions_search(projectId: int, data: schemas.SessionsSearchPayloadSchema = Body(...), context: schemas.CurrentContext = Depends(OR_context)): diff --git a/ee/api/chalicelib/core/sessions_replay.py b/ee/api/chalicelib/core/sessions_replay.py index 8aa0138b36..4bc4beac24 100644 --- a/ee/api/chalicelib/core/sessions_replay.py +++ b/ee/api/chalicelib/core/sessions_replay.py @@ -18,88 +18,6 @@ def __group_metadata(session, project_metadata): return meta -# for EE -# for backward compatibility -# This function should not use Clickhouse because it doesn't have `file_key` -def get_by_id2_pg(project_id, session_id, context: schemas.CurrentContext, full_data=False, - include_fav_viewed=False, group_metadata=False, live=True): - with pg_client.PostgresClient() as cur: - extra_query = [] - if include_fav_viewed: - extra_query.append("""COALESCE((SELECT TRUE - FROM public.user_favorite_sessions AS fs - WHERE s.session_id = fs.session_id - AND fs.user_id = %(userId)s), FALSE) AS favorite""") - extra_query.append("""COALESCE((SELECT TRUE - FROM public.user_viewed_sessions AS fs - WHERE s.session_id = fs.session_id - AND fs.user_id = %(userId)s), FALSE) AS viewed""") - query = cur.mogrify( - f"""\ - SELECT - s.*, - s.session_id::text AS session_id, - (SELECT project_key FROM public.projects WHERE project_id = %(project_id)s LIMIT 1) AS project_key, - encode(file_key,'hex') AS file_key - {"," if len(extra_query) > 0 else ""}{",".join(extra_query)} - {(",json_build_object(" + ",".join([f"'{m}',p.{m}" for m in metadata.column_names()]) + ") AS project_metadata") if group_metadata else ''} - FROM public.sessions AS s {"INNER JOIN public.projects AS p USING (project_id)" if group_metadata else ""} - WHERE s.project_id = %(project_id)s - AND s.session_id = %(session_id)s;""", - {"project_id": project_id, "session_id": session_id, "userId": context.user_id} - ) - cur.execute(query=query) - - data = cur.fetchone() - if data is not None: - data = helper.dict_to_camel_case(data) - if full_data: - if __is_mobile_session(data["platform"]): - data['events'] = events_mobile.get_by_sessionId(project_id=project_id, session_id=session_id) - for e in data['events']: - if e["type"].endswith("_IOS"): - e["type"] = e["type"][:-len("_IOS")] - elif e["type"].endswith("_MOBILE"): - e["type"] = e["type"][:-len("_MOBILE")] - data['crashes'] = events_mobile.get_crashes_by_session_id(session_id=session_id) - data['userEvents'] = events_mobile.get_customs_by_session_id(project_id=project_id, - session_id=session_id) - data['mobsUrl'] = [] - else: - data['events'] = events.get_by_session_id(project_id=project_id, session_id=session_id, - group_clickrage=True) - all_errors = events.get_errors_by_session_id(session_id=session_id, project_id=project_id) - data['stackEvents'] = [e for e in all_errors if e['source'] != "js_exception"] - # to keep only the first stack - # limit the number of errors to reduce the response-body size - data['errors'] = [errors_helper.format_first_stack_frame(e) for e in all_errors - if e['source'] == "js_exception"][:500] - data['userEvents'] = events.get_customs_by_session_id(project_id=project_id, - session_id=session_id) - data['domURL'] = sessions_mobs.get_urls(session_id=session_id, project_id=project_id, - check_existence=False) - data['mobsUrl'] = sessions_mobs.get_urls_depercated(session_id=session_id, check_existence=False) - # for EE - # context is required to check if the use have the right to access devtools - data['devtoolsURL'] = sessions_devtool.get_urls(session_id=session_id, project_id=project_id, - context=context, check_existence=False) - data['resources'] = resources.get_by_session_id(session_id=session_id, project_id=project_id, - start_ts=data["startTs"], duration=data["duration"]) - - data['notes'] = sessions_notes.get_session_notes(tenant_id=context.tenant_id, project_id=project_id, - session_id=session_id, user_id=context.user_id) - data['metadata'] = __group_metadata(project_metadata=data.pop("projectMetadata"), session=data) - data['issues'] = issues.get_by_session_id(session_id=session_id, project_id=project_id) - data['live'] = live and assist.is_live(project_id=project_id, session_id=session_id, - project_key=data["projectKey"]) - data["inDB"] = True - return data - elif live: - return assist.get_live_session_by_id(project_id=project_id, session_id=session_id) - else: - return None - - def get_pre_replay(project_id, session_id, context: schemas.CurrentContext): return { 'domURL': [sessions_mobs.get_first_url(project_id=project_id, session_id=session_id, check_existence=False)]} diff --git a/ee/api/or_dependencies.py b/ee/api/or_dependencies.py index 317f42fea1..081a74a21f 100644 --- a/ee/api/or_dependencies.py +++ b/ee/api/or_dependencies.py @@ -4,6 +4,7 @@ from fastapi import HTTPException, Depends from fastapi import Security +from fastapi.exceptions import RequestValidationError from fastapi.routing import APIRoute from fastapi.security import SecurityScopes from starlette import status @@ -33,6 +34,11 @@ async def custom_route_handler(request: Request) -> Response: logger.debug(f"call processed by: {self.methods} {self.path_format}") try: response: Response = await original_route_handler(request) + except RequestValidationError as exc: + # 422 validation exception + logger.warning(f"422 exception when calling: {request.method} {request.url}") + logger.warning(exc.errors()) + raise exc except HTTPException as e: if e.status_code // 100 == 4: return JSONResponse(content={"errors": e.detail if isinstance(e.detail, list) else [e.detail]}, @@ -43,6 +49,8 @@ async def custom_route_handler(request: Request) -> Response: if isinstance(response, JSONResponse): response: JSONResponse = response body = json.loads(response.body.decode('utf8')) + response.body = response.render(helper.cast_session_id_to_string(body)) + response.headers["Content-Length"] = str(len(response.body)) if response.status_code == 200 \ and body is not None and isinstance(body, dict) \ and body.get("errors") is not None: diff --git a/ee/api/routers/core_dynamic.py b/ee/api/routers/core_dynamic.py index f19345bce3..02b4c626e3 100644 --- a/ee/api/routers/core_dynamic.py +++ b/ee/api/routers/core_dynamic.py @@ -240,27 +240,6 @@ def get_projects(context: schemas.CurrentContext = Depends(OR_context)): recorded=True, user_id=context.user_id)} -# for backward compatibility -@app.get('/{projectId}/sessions/{sessionId}', tags=["sessions", "replay"], - dependencies=[OR_scope(Permissions.SESSION_REPLAY, ServicePermissions.SESSION_REPLAY)]) -def get_session(projectId: int, sessionId: Union[int, str], background_tasks: BackgroundTasks, - context: schemas.CurrentContext = Depends(OR_context)): - if not sessionId.isnumeric(): - return {"errors": ["session not found"]} - else: - sessionId = int(sessionId) - data = sessions_replay.get_by_id2_pg(project_id=projectId, session_id=sessionId, full_data=True, - include_fav_viewed=True, group_metadata=True, context=context) - if data is None: - return {"errors": ["session not found"]} - if data.get("inDB"): - background_tasks.add_task(sessions_viewed.view_session, project_id=projectId, user_id=context.user_id, - session_id=sessionId) - return { - 'data': data - } - - @app.post('/{projectId}/sessions/search', tags=["sessions"], dependencies=[OR_scope(Permissions.SESSION_REPLAY)]) def sessions_search(projectId: int, data: schemas.SessionsSearchPayloadSchema = Body(...),