Skip to content

Commit 2ef096d

Browse files
committed
refactor: move exception logging to crash handler for cleaner code
- Remove try/catch blocks from wrapper functions - Centralize exception logging in _crash_handler - Extract execution_id directly from request headers in crash handler - Temporarily set context when logging exceptions to ensure execution_id is included - This approach is cleaner and more similar to Flask's centralized exception handling
1 parent 2f81ad8 commit 2ef096d

File tree

2 files changed

+73
-48
lines changed

2 files changed

+73
-48
lines changed

src/functions_framework/aio/__init__.py

Lines changed: 72 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,48 @@
5454
_CRASH = "crash"
5555

5656

57-
async def _crash_handler(request, exc): # pragma: no cover
57+
async def _crash_handler(request, exc):
58+
import logging
59+
import traceback
60+
import json
61+
62+
# Log the exception
63+
logger = logging.getLogger()
64+
tb_lines = traceback.format_exception(type(exc), exc, exc.__traceback__)
65+
tb_text = ''.join(tb_lines)
66+
error_msg = f"Exception on {request.url.path} [{request.method}]\n{tb_text}".rstrip()
67+
68+
# Check if we need to set execution context for logging
69+
if _enable_execution_id_logging():
70+
# Get execution_id from request headers
71+
exec_id = request.headers.get(execution_id.EXECUTION_ID_REQUEST_HEADER)
72+
span_id = None
73+
74+
# Try to get span ID from trace context header
75+
trace_context = request.headers.get(execution_id.TRACE_CONTEXT_REQUEST_HEADER, "")
76+
trace_match = re.match(execution_id._TRACE_CONTEXT_REGEX_PATTERN, trace_context)
77+
if trace_match:
78+
span_id = trace_match.group("span_id") # pragma: no cover
79+
80+
if exec_id:
81+
# Temporarily set context for logging
82+
token = execution_id.execution_context_var.set(
83+
execution_id.ExecutionContext(exec_id, span_id)
84+
)
85+
try:
86+
# Output as JSON so LoggingHandlerAddExecutionId can process it
87+
log_entry = {"message": error_msg, "levelname": "ERROR"}
88+
logger.error(json.dumps(log_entry))
89+
finally:
90+
execution_id.execution_context_var.reset(token)
91+
else: # pragma: no cover
92+
# No execution ID, just log normally
93+
log_entry = {"message": error_msg, "levelname": "ERROR"}
94+
logger.error(json.dumps(log_entry))
95+
else:
96+
# Execution ID logging not enabled, log plain text
97+
logger.error(error_msg)
98+
5899
headers = {_FUNCTION_STATUS_HEADER_FIELD: _CRASH}
59100
return Response("Internal Server Error", status_code=500, headers=headers)
60101

@@ -107,36 +148,28 @@ def _http_func_wrapper(function, is_async, enable_id_logging=False):
107148
@execution_id.set_execution_context_async(enable_id_logging)
108149
@functools.wraps(function)
109150
async def handler(request):
110-
try:
111-
if is_async:
112-
result = await function(request)
151+
if is_async:
152+
result = await function(request)
153+
else:
154+
# TODO: Use asyncio.to_thread when we drop Python 3.8 support
155+
# Python 3.8 compatible version of asyncio.to_thread
156+
loop = asyncio.get_event_loop()
157+
result = await loop.run_in_executor(None, function, request)
158+
if isinstance(result, str):
159+
return Response(result)
160+
elif isinstance(result, dict):
161+
return JSONResponse(result)
162+
elif isinstance(result, tuple) and len(result) == 2:
163+
# Support Flask-style tuple response
164+
content, status_code = result
165+
if isinstance(content, dict):
166+
return JSONResponse(content, status_code=status_code)
113167
else:
114-
# TODO: Use asyncio.to_thread when we drop Python 3.8 support
115-
# Python 3.8 compatible version of asyncio.to_thread
116-
loop = asyncio.get_event_loop()
117-
result = await loop.run_in_executor(None, function, request)
118-
if isinstance(result, str):
119-
return Response(result)
120-
elif isinstance(result, dict):
121-
return JSONResponse(result)
122-
elif isinstance(result, tuple) and len(result) == 2:
123-
# Support Flask-style tuple response
124-
content, status_code = result
125-
if isinstance(content, dict):
126-
return JSONResponse(content, status_code=status_code)
127-
else:
128-
return Response(content, status_code=status_code)
129-
elif result is None:
130-
raise HTTPException(status_code=500, detail="No response returned")
131-
else:
132-
return result
133-
except Exception: # pragma: no cover
134-
# Log the exception while context is still active
135-
# The traceback will be printed to stderr which goes through LoggingHandlerAddExecutionId
136-
import sys
137-
import traceback
138-
traceback.print_exc(file=sys.stderr)
139-
raise
168+
return Response(content, status_code=status_code)
169+
elif result is None:
170+
raise HTTPException(status_code=500, detail="No response returned")
171+
else:
172+
return result
140173

141174
return handler
142175

@@ -154,22 +187,14 @@ async def handler(request):
154187
400, detail=f"Bad Request: Got CloudEvent exception: {repr(e)}"
155188
)
156189

157-
try:
158-
if is_async:
159-
await function(event)
160-
else:
161-
# TODO: Use asyncio.to_thread when we drop Python 3.8 support
162-
# Python 3.8 compatible version of asyncio.to_thread
163-
loop = asyncio.get_event_loop()
164-
await loop.run_in_executor(None, function, event)
165-
return Response("OK")
166-
except Exception: # pragma: no cover
167-
# Log the exception while context is still active
168-
# The traceback will be printed to stderr which goes through LoggingHandlerAddExecutionId
169-
import sys
170-
import traceback
171-
traceback.print_exc(file=sys.stderr)
172-
raise
190+
if is_async:
191+
await function(event)
192+
else:
193+
# TODO: Use asyncio.to_thread when we drop Python 3.8 support
194+
# Python 3.8 compatible version of asyncio.to_thread
195+
loop = asyncio.get_event_loop()
196+
await loop.run_in_executor(None, function, event)
197+
return Response("OK")
173198

174199
return handler
175200

@@ -277,7 +302,7 @@ def create_asgi_app(target=None, source=None, signature_type=None):
277302
)
278303

279304
exception_handlers = {
280-
500: _crash_handler,
305+
Exception: _crash_handler,
281306
}
282307
app = Starlette(routes=routes, exception_handlers=exception_handlers)
283308

tests/test_execution_id_async.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ async def test_async_set_execution_context_headers(
246246

247247
@pytest.mark.asyncio
248248
async def test_crash_handler_without_context_sets_execution_id():
249-
"""Test that crash handler returns proper error response."""
249+
"""Test that crash handler returns proper error response with crash header."""
250250
from functions_framework.aio import _crash_handler
251251
from unittest.mock import Mock
252252

0 commit comments

Comments
 (0)