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

Unhandled Exception on Request Failure to Resolve $ref #1053

Closed
firyalff opened this issue Mar 9, 2023 · 3 comments
Closed

Unhandled Exception on Request Failure to Resolve $ref #1053

firyalff opened this issue Mar 9, 2023 · 3 comments

Comments

@firyalff
Copy link

firyalff commented Mar 9, 2023

Background

No try catch on request call causing unhandled exception which can cause application error. See image below for server log on remote resolve error occurrence. In this case, one of ref API returns HTTP 502 on JSON Schema $ref resolve process

Screen Shot 2023-03-08 at 15 56 56

Proposal to Resolve

Looking at piece of code from this library on https://github.com/python-jsonschema/jsonschema/blob/main/jsonschema/validators.py#L975

try:
    import requests
except ImportError:
    requests = None

scheme = urlsplit(uri).scheme

if scheme in self.handlers:
    result = self.handlers[scheme](uri)
elif scheme in ["http", "https"] and requests:
    # Requests has support for detecting the correct encoding of
    # json over http
    result = requests.get(uri).json()
else:
    # Otherwise, pass off to urllib and assume utf-8
    with urlopen(uri) as url:
        result = json.loads(url.read().decode("utf-8"))

if self.cache_remote:
    self.store[uri] = result
return result

Try/Catch should be wrapping this part of code

result = requests.get(uri).json()

to ensure that error on request (e.g. 5xx HTTP response) can be handled gracefully.

If this is OK, I'd be glad to open PR for fix

@Julian
Copy link
Member

Julian commented Mar 9, 2023

Hello there! Thanks for the ticket.

There's a PR which is about to be released which will essentially be replacing the entire reference resolution implementation (deprecating RefResolver and replacing it). It's #1049 with the introduction of this new referencing library which is fully compliant and has APIs which I hope are a lot easier to understand and customize. (In case you didn't notice, at some point I transferred this issue between repositories).

The next release of jsonschema (v4.18.0) will contain a merged version of that PR, and should be released shortly in beta, and followed quickly by a regular release, assuming no critical issues are reported.

referencing will indeed allow you to make HTTP requests however you'd like to do so, including by catching requests and doing whatever you'd like if they error. I'd love it if you tried out the beta once it is released (likely in the next 2 weeks), or certainly it'd be hugely helpful to immediately install the branch containing this work (https://github.com/python-jsonschema/jsonschema/tree/referencing) and confirm. You can in the interim find documentation for the change in a preview page here -- a section specifically for hooking in HTTP clients is here and feedback is definitely welcome.

I'm going to close this given it will change in #1049 (wherein jsonschema will no longer do the retrieval at all, so yeah, you can do it how you'd like), but feel free to follow up with any comments. Let me know what you think!

@Julian Julian closed this as not planned Won't fix, can't repro, duplicate, stale Mar 9, 2023
@firyalff
Copy link
Author

Hello @Julian , thanks for the response. I've check the referencing library, that looks awesome since overriding reference resolve can be overridden with specific object. But, when will the referencing merged to master? As of now, is it safe to use it in production?

@Julian
Copy link
Member

Julian commented Mar 10, 2023

Great! Glad to hear. It'll likely be merged in the next week or so, and released in a beta at that moment, and finally in a full release shortly thereafter if a short time passes without critical issues.

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

No branches or pull requests

2 participants