Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: restore try-catch for HTML responses #8

Merged
merged 1 commit into from
May 25, 2023
Merged

fix: restore try-catch for HTML responses #8

merged 1 commit into from
May 25, 2023

Conversation

jpnurmi
Copy link
Contributor

@jpnurmi jpnurmi commented May 25, 2023

Partially reverts commit 2a4797a.

@jpnurmi
Copy link
Contributor Author

jpnurmi commented May 25, 2023

Reopened:

@jpnurmi jpnurmi merged commit dfb582e into canonical:main May 25, 2023
7 checks passed
@jpnurmi jpnurmi deleted the html branch May 25, 2023 11:37
try {
final json = jsonDecode(data);
if (json is Map && json['success'] == false) {
throw OdrsException(json['msg'] as String);
Copy link

@Feichtmeier Feichtmeier May 25, 2023

Choose a reason for hiding this comment

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

is null also a correct value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for json['msg']? i don't know but we can probably find the source code somewhere. :) did you receive such a response that had "success": false but no message since you're asking?

Choose a reason for hiding this comment

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

I just thought if json['success'] == null then the exception would not be thrown but if null is a correct value then this is np I guess :) ?
I didn't test the code yet, just looked over the commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm not sure if i'm looking at the correct code but technically, the error reporting helper function in odrs-web does seem to allow omitting the message.

def json_error(msg=None, errcode=400):
    """Error handler: JSON output"""
    item = {}
    item["success"] = False
    if msg:
        item["msg"] = msg
    dat = json.dumps(item, sort_keys=True, indent=4, separators=(",", ": "))
    return Response(response=dat, status=errcode, mimetype="application/json")

however, the helper method is not called without a message anywhere in the codebase... 🤔

odrs/util.py:def json_error(msg=None, errcode=400):
odrs/views_api.py:    json_error,
odrs/views_api.py:        return json_error(str(e))
odrs/views_api.py:            return json_error("invalid data, expected %s" % key)
odrs/views_api.py:            return json_error("missing data, expected %s" % key)
odrs/views_api.py:        return json_error("the user_hash is invalid")
odrs/views_api.py:        return json_error("summary is too long")
odrs/views_api.py:        return json_error("description is too long")
odrs/views_api.py:            return json_error("%s is not a valid string" % key)
odrs/views_api.py:            return json_error("account has been disabled due to abuse")
odrs/views_api.py:        return json_error("already reviewed this app")
odrs/views_api.py:        return json_error("summary is too short")
odrs/views_api.py:        return json_error("description is too short")
odrs/views_api.py:        return json_error("review contains taboo word")
odrs/views_api.py:        return json_error(str(e))
odrs/views_api.py:            return json_error("invalid data, expected %s" % key)
odrs/views_api.py:            return json_error("missing data, expected %s" % key)
odrs/views_api.py:        return json_error("the user_hash is invalid")
odrs/views_api.py:        return json_error("no user for {}".format(user_hash))
odrs/views_api.py:        return json_error(str(e))
odrs/views_api.py:            return json_error("invalid data, required %s" % key)
odrs/views_api.py:            return json_error("missing data, expected %s" % key)
odrs/views_api.py:        return json_error("the user_hash is invalid")
odrs/views_api.py:        return json_error("the user_skey is invalid")
odrs/views_api.py:            return json_error("account has been disabled due to abuse")
odrs/views_api.py:            return json_error("all negative karma used up")
odrs/views_api.py:        return json_error("invalid user_skey")
odrs/views_api.py:        return json_error("already voted on this app")
odrs/views_api.py:        return json_error("invalid review ID")
odrs/views_api.py:        return json_error(str(e))
odrs/views_api.py:            return json_error("invalid data, required %s" % key)
odrs/views_api.py:            return json_error("missing data, expected %s" % key)
odrs/views_api.py:        return json_error("the user_hash is invalid")
odrs/views_api.py:        return json_error("the user_skey is invalid")
odrs/views_api.py:        return json_error("no review")
odrs/views_api.py:        return json_error("no review")
odrs/views_api.py:        return json_error("the app_id is invalid")
odrs/views_api.py:        return json_error("invalid user_skey")

Choose a reason for hiding this comment

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

If you're not sure you could eventually also check the http response code to be 200.
Sorry if this is not helpful

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.

None yet

2 participants