Hello thanks for all your hard work on crystal!
I use the multi_auth shard which itself use oauth2 from the crystal stdlib.
the OAuth2::Error class is trying to decode the error from the payload expecting something like this:
json
{ "error": "some oauth error" }
This is what I understand according to the code in src/oauth2/error.cr:
pull.read_object do |key|
case key
when "error" then error = pull.read_string
when "error_description" then error_description = pull.read_string
else
raise "Unknown key in oauth2 error json: #{key}"
end
end
I suppose that must be the case with some oauth2 providers but this assumption is not valid for Facebook, which returns in case of error something like this:
{"error":{"message":"This authorization code has expired.","type":"OAuthException","code":100,"fbtrace_id":"GyzZxJWzHMP"}}
Because of error being an object instead of the string it makes this json reader crash with this error:
Unhandled exception: Expected string but was begin_object at 1:11 (JSON::ParseException)
from /usr/share/crystal/src/json/pull_parser.cr:518:5 in 'parse_exception'
from /usr/share/crystal/src/json/pull_parser.cr:510:5 in 'expect_kind'
from /usr/share/crystal/src/json/pull_parser.cr:180:5 in 'read_string'
from /usr/share/crystal/src/json/pull_parser.cr:83:5 in 'read_object_key'
from /usr/share/crystal/src/oauth2/error.cr:89:13 in 'new'
from /usr/share/crystal/src/json/from_json.cr:13:3 in 'from_json'
from /usr/share/crystal/src/oauth2/client.cr:154:7 in 'get_access_token_using_authorization_code'
[... more stack ]
I would have gladly fixed it myself but I'm a beginner crystal developer and I'm clearly not good enough yet to fix this issue.
Yes, this is stupid Facebook that doesn't respect standards:
error
REQUIRED. A single ASCII [USASCII] error code
I guess we'll have to change this.
We can probably make the class hold the JSON source as a string to be parsed, if needed, by clients. We can probably make the entire JSON string the error. I'll send a PR to fix this later today.
Most helpful comment
Yes, this is stupid Facebook that doesn't respect standards:
I guess we'll have to change this.
We can probably make the class hold the JSON source as a string to be parsed, if needed, by clients. We can probably make the entire JSON string the error. I'll send a PR to fix this later today.