Skip to content

Commit

Permalink
[Issue #2521] Have the error response always return an internal reque…
Browse files Browse the repository at this point in the history
…st ID (#2523)

## Summary
Fixes #2521

### Time to review: __3 mins__

## Changes proposed
Return the internal request ID in all error responses

## Context for reviewers
A follow-up from a discussion we had yesterday. This can aid in
debugging as a user can just give us the ID and we can easily look up
the corresponding logs.

Note `g` in Flask is a request-local global context (ie. each request
has their own separate `g`) which we use for storing a few other logging
related things already safely.
https://flask.palletsprojects.com/en/3.0.x/appcontext/#storing-data

## Additional information
<img width="1082" alt="Screenshot 2024-10-18 at 12 15 49 PM"
src="https://github.com/user-attachments/assets/e47874ed-8219-447c-947c-1c1bbd48f775">

<img width="715" alt="Screenshot 2024-10-18 at 12 16 09 PM"
src="https://github.com/user-attachments/assets/f3b921e4-f52f-4882-b017-5d3275100c7f">

<img width="558" alt="Screenshot 2024-10-18 at 12 17 05 PM"
src="https://github.com/user-attachments/assets/0ea12677-8632-46d6-bf1b-153ada50add6">

An example of the logs that show the internal ID - note I trimmed out
the full error message as it's a very long stack trace.
![Screenshot 2024-10-18 at 12 17
25 PM](https://github.com/user-attachments/assets/e923f84d-e295-4719-9d80-65ac98f13ca8)

---------

Co-authored-by: nava-platform-bot <platform-admins@navapbc.com>
  • Loading branch information
chouinar and nava-platform-bot authored Oct 23, 2024
1 parent 28a5a56 commit 45a677c
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 3 deletions.
4 changes: 4 additions & 0 deletions api/openapi.generated.yml
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,10 @@ components:
- object
allOf:
- $ref: '#/components/schemas/ValidationIssue'
internal_request_id:
type: string
description: An internal tracking ID
example: 550e8400-e29b-41d4-a716-446655440000
OpportunitySorting:
type: object
properties:
Expand Down
2 changes: 2 additions & 0 deletions api/src/api/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from typing import Any, Optional, Tuple

import apiflask
import flask

from src.api.schemas.extension import MarshmallowErrorContainer
from src.pagination.pagination_models import PaginationInfo
Expand Down Expand Up @@ -92,6 +93,7 @@ def restructure_error_response(error: apiflask.exceptions.HTTPError) -> Tuple[di
# we rename detail to data so success and error responses are consistent
"data": error.detail,
"status_code": error.status_code,
"internal_request_id": getattr(flask.g, "internal_request_id", None),
}
validation_errors: list[ValidationErrorDetail] = []

Expand Down
6 changes: 6 additions & 0 deletions api/src/api/schemas/response_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,9 @@ class ErrorResponseSchema(Schema):
errors = fields.List(
fields.Nested(ValidationIssueSchema()), metadata={"example": []}, dump_default=[]
)
internal_request_id = fields.String(
metadata={
"description": "An internal tracking ID",
"example": "550e8400-e29b-41d4-a716-446655440000",
}
)
9 changes: 6 additions & 3 deletions api/src/logging/flask_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,17 @@ def _add_global_context_info_to_log_record(record: logging.LogRecord) -> bool:


def _get_request_context_info(request: flask.Request) -> dict:
internal_request_id = str(uuid.uuid4())
flask.g.internal_request_id = internal_request_id

data = {
"request.id": request.headers.get("x-amzn-requestid", ""),
"request.method": request.method,
"request.path": request.path,
"request.url_rule": str(request.url_rule),
# A backup ID in case the x-amzn-requestid isn't passed in
# doesn't help with tracing across systems, but at least links within a request
"request.internal_id": str(uuid.uuid4()),
# This ID is used to group all logs for a given request
# and is returned in the API response for any 4xx/5xx scenarios
"request.internal_id": internal_request_id,
}

# Add query parameter data in the format request.query.<key> = <value>
Expand Down
1 change: 1 addition & 0 deletions api/tests/src/api/test_healthcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ def err_method(*args):
response = client.get("/health")
assert response.status_code == 503
assert response.get_json()["message"] == "Service Unavailable"
assert response.get_json()["internal_request_id"] is not None

0 comments on commit 45a677c

Please sign in to comment.