Skip to content

Conversation

simonLeary42
Copy link
Collaborator

@simonLeary42 simonLeary42 commented Sep 22, 2025

This is only a slight improvement for #306. In general, I think it makes sense for "forbidden" and "bad request" errors to also make an error popup and generate an error ID.

  • moved the user-facing error message functionality from the shutdown function to UnitySite::errorToUser
  • activated user-facing error message functionality in badRequest, forbidden
  • extended UnitySite::errorLog to take new parameters errorid, error, data
    • before, errorid was only printed from the shutdown function, and it was assumed that the reader would look at the previous message from errorLog to correlate it with the errorid
    • data parameter is used by the shutdown function since error_get_last doesn't return a Throwable but just an array of error info
  • extended badRequest, forbidden to take new parameters error, data
    • not yet used, should prove useful
    • ArrayKeyException is one good use for the error for badRequest
  • created UnitySite::throwableToArray for nice logging of thrown errors/exceptions
  • removed json pretty-printing from phpopenldaper
  • enabled the JSON_UNESCAPED_SLASHES flag
  • split stack traces by newline for more readable JSON output
  • replaced the enable_shutdown_message config option with a new enable_error_to_user config option
  • replaced UnitySite::headerResponseCode($code, $reason) with http_response_code()

example: uncaught exception

before

error log:

[Mon Sep 22 17:27:22.669519 2025] [php:notice] [pid 11] [client 192.168.65.1:26058] internal server error: {"message":"{\\"type\\":1,\\"message\\":\\"Uncaught Exception in \\\\\\/var\\\\\\/www\\\\\\/unity-web-portal\\\\\\/webroot\\\\\\/panel\\\\\\/new_account.php:72\\\\nStack trace:\\\\n#0 {main}\\\\n thrown\\",\\"file\\":\\"\\\\\\/var\\\\\\/www\\\\\\/unity-web-portal\\\\\\/webroot\\\\\\/panel\\\\\\/new_account.php\\",\\"line\\":72,\\"unity_error_id\\":\\"68d186faa3727\\"}","REMOTE_USER":"[email protected]","REMOTE_ADDR":"192.168.65.1","trace":"#0 \\/var\\/www\\/unity-web-portal\\/resources\\/lib\\/UnitySite.php(89): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()\\n#1 [internal function]: UnityWebPortal\\\\lib\\\\UnitySite::shutdown()\\n#2 {main}"}, referer: http://127.0.0.1:8000/

json reformatted:

{
    "message": "{\\"type\\":1,\\"message\\":\\"Uncaught Exception in \\\\\\/var\\\\\\/www\\\\\\/unity-web-portal\\\\\\/webroot\\\\\\/panel\\\\\\/new_account.php: 72\\\\nStack trace:\\\\n#0 {main}\\\\n  thrown\\",\\"file\\":\\"\\\\\\/var\\\\\\/www\\\\\\/unity-web-portal\\\\\\/webroot\\\\\\/panel\\\\\\/new_account.php\\",\\"line\\":72,\\"unity_error_id\\":\\"68d186faa3727\\"}",
    "REMOTE_USER": "[email protected]",
    "REMOTE_ADDR": "192.168.65.1",
    "trace": "#0 \\/var\\/www\\/unity-web-portal\\/resources\\/lib\\/UnitySite.php(89): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()\\n#1 [internal function]: UnityWebPortal\\\\lib\\\\UnitySite::shutdown()\\n#2 {main}"
}

after

error log:

[Mon Sep 22 17:25:53.470629 2025] [php:notice] [pid 11] [client 192.168.65.1:30990] internal server error: {"message":"An internal server error has occurred.","REMOTE_USER":"[email protected]","REMOTE_ADDR":"192.168.65.1","errorid":"68d186a172e31","trace":["#0 /var/www/unity-web-portal/resources/lib/UnitySite.php(122): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()","#1 /var/www/unity-web-portal/resources/lib/UnitySite.php(138): UnityWebPortal\\\\lib\\\\UnitySite::internalServerError()","#2 [internal function]: UnityWebPortal\\\\lib\\\\UnitySite::shutdown()","#3 {main}"],"data":{"error":{"type":1,"message":["Uncaught Exception in /var/www/unity-web-portal/webroot/panel/new_account.php:72","Stack trace:","#0 {main}"," thrown"],"file":"/var/www/unity-web-portal/webroot/panel/new_account.php","line":72}}}, referer: http://127.0.0.1:8000/index.php

json reformatted:

{
    "message": "An internal server error has occurred.",
    "REMOTE_USER": "[email protected]",
    "REMOTE_ADDR": "192.168.65.1",
    "errorid": "68d186a172e31",
    "trace": [
        "#0 /var/www/unity-web-portal/resources/lib/UnitySite.php(122): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()",
        "#1 /var/www/unity-web-portal/resources/lib/UnitySite.php(138): UnityWebPortal\\\\lib\\\\UnitySite::internalServerError()",
        "#2 [internal function]: UnityWebPortal\\\\lib\\\\UnitySite::shutdown()",
        "#3 {main}"
    ],
    "data": {
        "error": {
            "type": 1,
            "message": [
                "Uncaught Exception in /var/www/unity-web-portal/webroot/panel/new_account.php:72",
                "Stack trace:",
                "#0 {main}",
                "  thrown"
            ],
            "file": "/var/www/unity-web-portal/webroot/panel/new_account.php",
            "line": 72
        }
    }
}

example: bad request

before

error log:

[Mon Sep 22 17:36:26.948241 2025] [php:notice] [pid 16] [client 192.168.65.1:64646] bad request: {"message":"The selected PI 'foobar'does not exist","REMOTE_USER":"[email protected]","REMOTE_ADDR":"192.168.65.1","trace":"#0 \\/var\\/www\\/unity-web-portal\\/resources\\/lib\\/UnitySite.php(61): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()\\n#1 \\/var\\/www\\/unity-web-portal\\/webroot\\/panel\\/new_account.php(29): UnityWebPortal\\\\lib\\\\UnitySite::badRequest()\\n#2 {main}"}, referer: http://127.0.0.1:8000/panel/new_account.php

json reformatted:

{
    "message": "The selected PI 'foobar'does not exist",
    "REMOTE_USER": "[email protected]",
    "REMOTE_ADDR": "192.168.65.1",
    "trace": "#0 \\/var\\/www\\/unity-web-portal\\/resources\\/lib\\/UnitySite.php(61): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()\\n#1 \\/var\\/www\\/unity-web-portal\\/webroot\\/panel\\/new_account.php(29): UnityWebPortal\\\\lib\\\\UnitySite::badRequest()\\n#2 {main}"
}

after

error log:

[Mon Sep 22 17:37:38.080828 2025] [php:notice] [pid 11] [client 192.168.65.1:42080] bad request: {"message":"The selected PI 'foobar'does not exist","REMOTE_USER":"[email protected]","REMOTE_ADDR":"192.168.65.1","errorid":"68d1896213b8a","trace":["#0 /var/www/unity-web-portal/resources/lib/UnitySite.php(106): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()","#1 /var/www/unity-web-portal/webroot/panel/new_account.php(29): UnityWebPortal\\\\lib\\\\UnitySite::badRequest()","#2 {main}"]}, referer: http://127.0.0.1:8000/panel/new_account.php

json reformatted:

{
    "message": "The selected PI 'foobar'does not exist",
    "REMOTE_USER": "[email protected]",
    "REMOTE_ADDR": "192.168.65.1",
    "errorid": "68d1896213b8a",
    "trace": [
        "#0 /var/www/unity-web-portal/resources/lib/UnitySite.php(106): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()",
        "#1 /var/www/unity-web-portal/webroot/panel/new_account.php(29): UnityWebPortal\\\\lib\\\\UnitySite::badRequest()",
        "#2 {main}"
    ]
}

@simonLeary42 simonLeary42 force-pushed the better-error branch 5 times, most recently from 6c042fb to 1340a9f Compare September 22, 2025 15:51
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves error handling and logging by consolidating user-facing error messages into a centralized system and enhancing the error logging functionality with additional metadata.

  • Refactored user-facing error message logic from shutdown function to centralized UnitySite::errorToUser method
  • Enhanced error logging with additional parameters for error IDs, exception objects, and data payloads
  • Replaced enable_shutdown_message config option with enable_error_to_user for better clarity

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
resources/lib/phpopenldaper Updated submodule commit to remove JSON pretty-printing
resources/lib/UnitySite.php Core refactoring with new centralized error handling and enhanced logging
deployment/overrides/worker/config/config.ini Updated config option name for worker environment
deployment/overrides/phpunit/config/config.ini Updated config option name for test environment
defaults/config.ini.default Updated default config option name

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@simonLeary42 simonLeary42 mentioned this pull request Sep 22, 2025
@simonLeary42 simonLeary42 force-pushed the better-error branch 2 times, most recently from 19f216f to a851f50 Compare September 22, 2025 16:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@simonLeary42 simonLeary42 force-pushed the better-error branch 5 times, most recently from cb9db0c to 4b7ca30 Compare September 22, 2025 17:23
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

not in this version of PHP

no args for die

replace enable_shutdown_msg with enable_error_to_user

only make popup if content already sent

prettier conditionals

revert die

shorten

rearrange

Apply suggestion from @Copilot

Co-authored-by: Copilot <[email protected]>

Apply suggestion from @Copilot

Co-authored-by: Copilot <[email protected]>

Apply suggestion from @Copilot

Co-authored-by: Copilot <[email protected]>

Apply suggestion from @Copilot

Co-authored-by: Copilot <[email protected]>

use status code for redirect

fix type, revert change
@simonLeary42 simonLeary42 merged commit 434c6fd into main Sep 22, 2025
2 checks passed
@simonLeary42 simonLeary42 deleted the better-error branch September 22, 2025 17:42
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.

1 participant