Skip to content

LTI1p3: drop detailed logging to DEBUG level - #1527

Merged
bnmnetp merged 1 commit into
RunestoneInteractive:mainfrom
ascholerChemeketa:lti-debug-drop
Sep 22, 2026
Merged

bnmnetp merged 1 commit into
RunestoneInteractive:mainfrom
ascholerChemeketa:lti-debug-drop

Conversation

@ascholerChemeketa

Copy link
Copy Markdown
Contributor

Drops the new detailed logging for LTI1p3 to DEBUG level. That should keep it out of production but keep it available in dev instances.

Copilot AI lite review requested due to automatic review settings September 20, 2026 16:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The updated logging introduces/retains error-path issues (exception handling when no response exists) and continues to log potentially sensitive data via r.__dict__/raw bodies.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adjusts LTI 1.3 service-connector logging so verbose HTTP failure details are emitted at DEBUG level, keeping production logs quieter while preserving diagnostics for development.

Changes:

  • Downgrades several get_access_token() failure logs from ERROR to DEBUG and reduces the amount of response detail logged.
  • Downgrades make_service_request() failure logging from ERROR to DEBUG (while keeping detailed response logging).
File Description
components/​rsptx/​lti1p3/​pylti1p3/​service_connector.py Moves LTI1p3 HTTP failure logging to DEBUG and adjusts what response details are logged.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 121 to 125
except Exception:
raw_body = await r.text()
rslogger.error(
f"get_access_token exception caught: {r.status} - {r.reason}. Headers: {r.headers}. Full response: {r.__dict__}"
rslogger.debug(
f"get_access_token exception caught: {r.status} - {r.reason}. Headers: {r.headers}."
)
@ascholerChemeketa

Copy link
Copy Markdown
Contributor Author

Fixed the unused raw_body via force push. Pretty sure there is no way to have None in those flagged spots in practice.

@bnmnetp
bnmnetp merged commit 1b853a5 into RunestoneInteractive:main Sep 22, 2026
1 check 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.

3 participants