Catching RefResolutionError no longer works

[ikonst]

With 4.18, reference resolution errors become referencing.exceptions.Unresolvable, so existing code which catches jsonschema.exceptions.RefResolutionError would no longer handle them.

Might want to consider wrapping them at the "jsonschema boundary", since it's a bit of a breaking change for a minor version.

Notably RefResolutionError is raised even in naive cases when a user never passes in a RefResolver (that's been deprecated) knowingly, so there's no warning to the user that will make them reckon with the deprecation.

jsonschema.validate(
    instance={},
    schema={
        'type': 'object',
        '$ref': '#/$defs/bogus',
    },
)

[Julian]

I considered this at one point (and decided against it only when I realized how non-trivial it is to do this). I can revisit and try again now that things are slightly more finished, though my rationalization as I was playing with this is that in general in my experience it's not really considered a backwards incompatible change to raise new exceptions from an existing code block, especially when recovering from the exception isn't really feasible anyhow like in this case -- i.e. I don't think it'd be unreasonable if jsonschema.validate started raising some third type of exception for some other error case -- right now the only thing that's been firmly promised is that ValidationError is the kind of error you get for validation errors, but you can get all sorts of other exceptions if you have an invalid schema (and RefResolutionError is a specific kind of invalid schema).

So yeah willing to be convinced I suppose, I can certainly see an argument for this -- or certainly to review a PR, and possibly will have another look myself after resolving the more "pressing" alpha issues like #1065. But it's not 100% clear to me yet that this is a necessary thing to preserve, so I don't suspect it'll block the release if everything else is resolved and it turns out it's indeed annoying to do this.

[robherring]

FWIW, I (https://github.com/devicetree-org/dt-schema) also depend on and raise RefResolutionError exceptions in my custom handler. However, things currently appear to be broken beyond that, so I'll probably just pin things at <4.18 until I move to the new API. The bigger issue seems to be schema1 ref to schema2 to local (schema2) ref breaks on the local ref. I haven't debugged it further.

[Julian]

Happy to have a look if you can share more details. The original ticket here I certainly understood to be about catching RefResolutionErrors during validation. You're raising them? That's certainly not something I intended to be public API, it's not usually the case that you can rely on raising exceptions from a library (and ValidationError is an outlier there I think), but I'm happy to look at an example.

The bigger issue seems to be schema1 ref to schema2 to local (schema2) ref breaks on the local ref. I haven't debugged it further.

I can't visualize this (and whether it has to do with the above) -- it sounds like it doesn't though -- and that this might be something separate? An actual chain of references that you have that's now broken?

[Julian]

Oh I see, you're talking about in a handler for RefResolvers? That entire interface is deprecated now, so I suspect that's not related to this ticket? You should be able to keep using that exactly as is (without even pinning?) until you move to the new APIs, as nothing's changed there I don't think. If it has I'd like to know, but I don't think that's related to this ticket (which yeah is AFAIU about catching RefResolutionError for some reason).

[robherring]

Oh I see, you're talking about in a handler for RefResolvers?

Yes, that's where I raise them. I also catch them, so I think it is related. Perhaps with my custom RefResolver, I'm the only source. Not really sure. If the exception type is changing, I think that will need an update on my side and 4.18 won't just work.

[Julian]

That should only be the case if you're not using RefResolver. If you are (which you must be if you're configuring handlers), it shouldn't affect you -- if it does definitely file a bug!

(All the internal RefResoler-using code is AFAIR basically completely unchanged, so it should still raise and catch the same exceptions as before once you use RefResolver).

[Julian]

This actually may not have been so difficult in the end. The referenced commit will go out in another alpha (a4) in a few minutes. Try it out, and let me know if it doesn't fix whatever existing code you had that was assuming a RefResolutionError was what you'd get from the failed resolution.

[Julian]

(Obviously as I say it's still advisable to move to catching Unresolvable as RefResolutionError is still deprecated, but now at least you'll get a warning not an error hopefully).