[PR #4317] [MERGED] Modify HttpFetchException flow to log the exception #6343

Closed
opened 2026-02-05 10:29:49 +03:00 by OVERLORD · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/BookStackApp/BookStack/pull/4317
Author: @devdot
Created: 6/16/2023
Status: Merged
Merged: 6/20/2023
Merged by: @ssddanbrown

Base: developmentHead: http-fetch-improve-exception-logging


📝 Commits (2)

  • c35080d Modify HttpFetchException handle to log exception
  • 97d46f4 Revert some changes to HttpFetchException

📊 Changes

3 files changed (+31 additions, -5 deletions)

View changed files

📝 app/Uploads/HttpFetcher.php (+2 -1)
📝 app/Uploads/UserAvatars.php (+4 -4)
📝 tests/Uploads/AvatarTest.php (+25 -0)

📄 Description

Currently, HttpFetchException is caught and logged, but without the exception (that would contain the actual curl error):

ec775aec02/app/Uploads/UserAvatars.php (L46-L53)

In that helper method, the exception is caught and re-thrown, but without the original exception as previous exception:

ec775aec02/app/Uploads/UserAvatars.php (L112-L121)

The proposed change is to add the exception to the logged message for improved debugging, and to make the exception a PrettyException for when it's shown to the user. By using PrettyException, the details property is available to show the original (technical) error message, although that may be removed if it should rather not be shown to the user.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/BookStackApp/BookStack/pull/4317 **Author:** [@devdot](https://github.com/devdot) **Created:** 6/16/2023 **Status:** ✅ Merged **Merged:** 6/20/2023 **Merged by:** [@ssddanbrown](https://github.com/ssddanbrown) **Base:** `development` ← **Head:** `http-fetch-improve-exception-logging` --- ### 📝 Commits (2) - [`c35080d`](https://github.com/BookStackApp/BookStack/commit/c35080d6cef5b5600cb4f93ca2fd25d0f31a4cd2) Modify HttpFetchException handle to log exception - [`97d46f4`](https://github.com/BookStackApp/BookStack/commit/97d46f43a76db9daed2692e73af613fad6e0363d) Revert some changes to HttpFetchException ### 📊 Changes **3 files changed** (+31 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `app/Uploads/HttpFetcher.php` (+2 -1) 📝 `app/Uploads/UserAvatars.php` (+4 -4) 📝 `tests/Uploads/AvatarTest.php` (+25 -0) </details> ### 📄 Description Currently, `HttpFetchException` is caught and logged, but without the exception (that would contain the actual curl error): https://github.com/BookStackApp/BookStack/blob/ec775aec02c0887d5cf2dc23c938a75b7eaf67d2/app/Uploads/UserAvatars.php#L46-L53 In that helper method, the exception is caught and re-thrown, but without the original exception as previous exception: https://github.com/BookStackApp/BookStack/blob/ec775aec02c0887d5cf2dc23c938a75b7eaf67d2/app/Uploads/UserAvatars.php#L112-L121 The proposed change is to add the exception to the logged message for improved debugging, and to make the exception a `PrettyException` for when it's shown to the user. By using `PrettyException`, the details property is available to show the original (technical) error message, although that may be removed if it should rather not be shown to the user. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-05 10:29:49 +03:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/BookStack#6343