JsonLdUrl.resolve returns null in case of URI parse error

[djodjoni]

in public static String resolve(String baseUri, String pathToResolve) we have

try {
            URI uri = new URI(baseUri);
            // query string parsing
            if (pathToResolve.startsWith("?")) {
                // drop fragment from uri if it has one
                if (uri.getFragment() != null) {
                    uri = new URI(uri.getScheme(), uri.getAuthority(), uri.getPath(), null, null);
                }
                // add query to the end manually (as URI.resolve does it wrong)
                return uri.toString() + pathToResolve;
            }

            uri = uri.resolve(pathToResolve);
            // java doesn't discard unnecessary dot segments
            String path = uri.getPath();
            if (path != null) {
                path = JsonLdUrl.removeDotSegments(uri.getPath(), true);
            }
            return new URI(uri.getScheme(), uri.getAuthority(), path, uri.getQuery(),
                    uri.getFragment()).toString();
        } catch (final URISyntaxException e) {
            return null;
        }

This results in hiding original exception and significantly increasing the debugging efforts.

In the case when base URI is set by JsonLdOptions the error may come very late with the message "loading remote context failed".

I think it makes sense to throw immediately when URI creation fails. Also it could be nice to immediately convert to URI when base set from JsonLdOptions and fail if URI not good.

Shall I make a PR for that ?

[ansell]

I don't mind if you submit a PR to rearchitect that method to throw exceptions, either a JsonLdError or just rethrowing the URISyntaxException. It will require other changes to add try-catch to code that is using that method, as they should all be assuming that null will be part of their control flow and dealing with it appropriately

[djodjoni]

as they should all be assuming that null will be part of their control flow and dealing with it appropriately

I don't think this is the case. Jena for example throws wrapped NPE some time later and it wasn't trivial to debug. Most of the times one expects to get something out of a resolve or fail. One could argue that null is acceptable when the URI is not well formatted but in the context of linked data I don't think this is the case. For example documents with bad URIs can be parsed(if the @context is local) but when reasoning is applied it will fail.

I don't mind if you submit a PR to rearchitect that method to throw exceptions, either a JsonLdError or just rethrowing the URISyntaxException

Although I am in favour of fault tolerance I don't particularly like checked exceptions as JsonLdError and URISyntaxException. The method could throw IllegalArgumentException and NullPointerException as does URI.resolve and URI.create, infact an easy refactoring would be to change new URI() with URI.create() this will solve the issue with the method signature.

[ansell]

Given the method is fairly central, I would like to see some concrete ideas in a pull request to get a feel for how broad the effect will be, and to compare it to the JSON-LD specs to see if the error handling matches the general flow of the algorithms in the API spec. Your ideas sound good, but would like to have some code to work with the decide.

[djodjoni]

Ok will do that over the weekend.