This is supposed to be readable in HTML, but I doubt the backtrace will be displayed in a readable manner. Perhaps, this should be wrapped in a <pre> tag or smth alike?

Good catch, I'll look into that.

Semi-related tangent so feel free to disregard: Is it on anyone's radar to refactor this function at all? I saw "Deprecate the error_prepend_string and error_append_string INI directives" in https://wiki.php.net/rfc/deprecations_php_8_5, which would help make this a little more readable, but I wonder if there's more we could do simplify this logic. I acknowledge that is probably easier said than done, however.

I doubt refactoring this function is on someone's TODO list. There are indeed some outdated pieces (e.g. the ones you mentioned but also xmlrpc_errors imo). However, the question with refactoring is always how much it's worth spending time on this. This function likely is not on any hot code path and refactoring also brings in a risk of breaking things 🤷

would be also nice to break up those huge long lines when you are in it...

@nielsdos apologies, but what's the best way to wrap a zend_string in <pre>? strpprintf? Or would it be the worst thing to just change this to %s%s%s and then pass in ZSTR_LEN(backtrace) ? "<pre>Stack trace:\n" : "", ZSTR_VAL(backtrace), ZSTR_LEN(backtrace) ? "</pre>" : ""? We could also drop adding the pre at all since we already have some fatal errors with stack traces that do not have pre: https://github.com/search?q=repo%3Aphp%2Fphp-src+%22%3Cb%3EFatal+error%3C%2Fb%3E%22&type=code, e.g.

<br />
<b>Fatal error</b>:  Uncaught Error: Cannot access private property test::$pri in %s004.test.php:8
Stack trace:
#0 {main}
  thrown in <b>%s004.test.php</b> on line <b>8</b><br />

@bukka I received feedback elsewhere to not break up long lines, though maybe here is a good counterpoint. I'd personally like to err on keeping it as-is since I'd like to be able to merge this relatively soon, but if people feel strongly I could break this up. This is kind of what I was getting at with my earlier comment - this function could probably be simplified to make it more readable, besides just the long line length.

If we wrap it with pre it should be done consistently.
If I were to write the code, I would go for the ZSTR_LEN() ternary trick.

If we wrap it with pre it should be done consistently.

I'm not sure I follow - what I was saying was that we currently do not wrap it with <pre> (for errors that already have backtraces before my change), so if we wrap it with <pre> here we'd be inconsistent with existing errors like:

If I were to write the code, I would go for the ZSTR_LEN() ternary trick.

Thanks, I was leaning that way but I wanted your opinion. I'm still not convinced we should add the <pre> based on the above, but if you think we should I'll go the ZSTR_LEN route.

I received feedback elsewhere to not break up long lines,

In case you are referring to my remark here: #16937 (comment) - this case of a printf-style function is quite different and splitting it across multiple lines would be reasonable, because it accurately represents the complexity of the statement.

Ok let's not do the <pre> conversion.

@TimWolla I was! and when I said "maybe here is a good counterpoint", I meant that I could see that you might agree with splitting this up. My preference was to not do this myself though, since for consistency I should probably do that for all of them, and I was concerned with making it a topic for bikeshedding.