ErikMinekus / sm-ripext

SourceMod REST in Pawn Extension
https://forums.alliedmods.net/showthread.php?t=298024
GNU General Public License v3.0
136 stars 38 forks source link

non-JSON response causes the exception #31

Closed CrazyHackGUT closed 3 years ago

CrazyHackGUT commented 3 years ago

In oldest RiP version (1.0.7 for example), if web server returned a non-JSON response, RiP just returns null when we try access to HTTPResponse.Data. In newest versions, this behavior is changed: if web server is returned a non-JSON response, RiP throws exception Invalid JSON in line -1, column -1: wrong arguments. This "breaking change" is undocumented nowhere.

This is expected change?

ErikMinekus commented 3 years ago

Yes, this is intentional. It makes it clear that the server returned invalid JSON, so the extension can't handle it. I will add an @error doc to HTTPResponse.Data for it.

CrazyHackGUT commented 3 years ago

For what purposes in ripext-test.sp implemented validation of HTTPResponse.Data property on nullability, if in all cases now if response is not provided, extension throws exception? https://github.com/ErikMinekus/sm-ripext/blob/ebad303ca993b326ab1a60e4298b9d296bccfca8/http_natives.cpp#L428-L449 This is really required now?

Wend4r commented 3 years ago

The SourcePawn shell should have functions that can check by bypassing exceptions and taking them under the control. I understand that you want to abstract everything in JSON. But it is not always convenient for the remote server to give only it. I want to suggest adding a property to check whether the response is JSON.

ErikMinekus commented 3 years ago

@Wend4r REST in Pawn sends the Accept: application/json header, so if the API cannot return JSON, it should return a 406 Not Acceptable status, which you can already check for.

If the API sends a 2xx success status, but the body cannot be parsed as JSON, you would want to know whether it's because the body does not contain JSON at all, or whether it's invalid JSON. But since we can't catch exceptions, I can look into another way to retrieve the error.

Wend4r commented 3 years ago

Thank you. I understand now