[pve-devel] [RFC PATCH http-server 1/2] add error message into http body

[Dominik Csapak]

On 1/15/25 17:08, Thomas Lamprecht wrote:
> Am 08.01.25 um 09:45 schrieb Dominik Csapak:
>> In our rust client, we can't access the http reason phrases[0], so let's
>> put them into the body itself if we don't specify an explicit content.
>>
>> our proxmox-client code in rust already uses the body as message if
>> there is one [1], so we get that automatically.
>>
>> 0: https://github.com/hyperium/http/issues/737
>> 1: https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox-client/src/client.rs;h=9b078a9820405b22ca54c17ea4da4c586e0649b4;hb=refs/heads/master#l237
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   src/PVE/APIServer/AnyEvent.pm | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
>> index 24209a1..bd76488 100644
>> --- a/src/PVE/APIServer/AnyEvent.pm
>> +++ b/src/PVE/APIServer/AnyEvent.pm
>> @@ -388,6 +388,7 @@ sub error {
>>       my ($self, $reqstate, $code, $msg, $hdr, $content) = @_;
>>   
>>       eval {
>> +	$content //= $msg; # write error into body by default
> 
> might this need altering the content-type or is that already to an
> OK default for a plain string? Just not that it's set to, e.g.,
> application/json but contains the error that cannot be parsed
> as JSON.
> 
> I can look myself, but I thought you might have already done so when
> developing this.
> 
> If that's fine I'd have nothing against applying this now.

The newly created response here is not setting an explicit content-type, and
according to the relevant part of the http spec[0], the client may assume
'application/octet-stream' or may try to identify the content-type
from the content itself.

While I guess most of the time we put strings in the error, it may happen
that this is something different here (since we sometimes simply
pass through the message from 'die') so explicitly setting
to text/plain could also be sometimes wrong here.

So I'd either leave it like this, or reformat the message to be e.g.
always text in some form, e.g. with

if (!defined($content)) {
     $content = "Some error occured: $msg";
     # set content type here
}

0: https://www.rfc-editor.org/rfc/rfc9110.html#name-content-type

> 
>>   	my $resp = HTTP::Response->new($code, $msg, $hdr, $content);
>>   	$self->response($reqstate, $resp);
>>       };
>