schema-registry icon indicating copy to clipboard operation
schema-registry copied to clipboard

Improve POST /Groups Validation Feedback

Open tsteinholz opened this issue 2 years ago • 2 comments

Tested Versions:

  • pravega/schemaregistry:0.2.0
  • pravega/schemaregistry:0.4.0

Problem description

POST: http://{schema registry uri}:9092/v1/groups

{
   "groupName": "mygroup",
   "groupProperties": {
      "serializationFormat":{
         "serializationFormat":"Json"
      },
      "compatibility":{
         "policy":"BackwardTransitive"
      },
      "allowMultipleTypes":false,
      "properties":{ }
   }
}

HTTP 400 Error when creating a group using the "Json" serialization format. Server Output: [grizzly-http-server-0] WARN i.p.s.s.r.r.AbstractResource - Bad argument for request createGroup failed with exception: . null. {}

HTTP 500 Error when creating a group with a "JSON" or "json" serialization format.

2022-09-06 17:46:22,042 278827 [RestServer RUNNING] WARN  i.p.s.s.r.r.AbstractResource - Request createGroup failed with exception:  failed with Internal Server error.
java.lang.NullPointerException: null
	at io.pravega.schemaregistry.contract.transform.ModelHelper.decode(ModelHelper.java:65)
	at io.pravega.schemaregistry.contract.transform.ModelHelper.decode(ModelHelper.java:251)
	at io.pravega.schemaregistry.server.rest.resources.GroupResourceImpl.lambda$createGroup$10(GroupResourceImpl.java:138)
	at io.pravega.schemaregistry.server.rest.resources.AbstractResource.lambda$withAuthorization$1(AbstractResource.java:77)
	at java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(Unknown Source)

Problem location

API Rest Controller for Groups (POST), possibility the i.p.s.s.r.r.AbstractResource.

Suggestions for an improvement

The user gets no feedback in the HTTP return as to what happened, only the return code. There is an unhandled exception when using invalid serialization formats and it is not clear to me if the schema registry is able to create schemas with the JSON serialization format or if there is another validation error preventing the group from being defined.

The current payload is based of the (invalid) example provided here https://pravega.io/docs/snapshot/schema-registry/rest-usage

tsteinholz avatar Sep 06 '22 17:09 tsteinholz

I found the error was coming from the following line of code throwing an IllegalArgumentException with no message in it. Resulting in the "null. {}" message being returned to the user.

        Preconditions.checkArgument(isValidCompatibilityForFormat(groupProperties.getSerializationFormat(), groupProperties.getCompatibility()));

src: https://github.com/pravega/schema-registry/blob/master/server/src/main/java/io/pravega/schemaregistry/service/SchemaRegistryService.java#L143

The source of this issue seems to be the inconsistent usage of the isValidCompatibilityForFormat method.

    private boolean isValidCompatibilityForFormat(SerializationFormat serializationFormat, Compatibility compatibility) {
        switch (serializationFormat) {
            case Avro:
                return true;
            case Protobuf:
                // Only Allow Any or Deny All are allowed values. 
            case Json:
                // Only Allow Any or Deny All are allowed values. 
            case Custom:
                // Only Allow Any or Deny All are allowed values. 
            case Any:
                return compatibility.getType().equals(Compatibility.Type.AllowAny) || compatibility.getType().equals(Compatibility.Type.DenyAll);
            default:
                throw new IllegalArgumentException("Unknown serialization format");
        }
    }

src: https://github.com/pravega/schema-registry/blob/0072abf26e6e09dbe30ab246933e84e6e3b18bdf/server/src/main/java/io/pravega/schemaregistry/service/SchemaRegistryService.java#L647

It is documented to return a boolean if the schema is valid, however, it also throws an IllegalArgumentException with a description.

Since the method is being called inside of a Preconditions.checkArgument the IllegalArgumentException with a description ("Unknown serialization format") is actually resulting in an HTTP 500 error to the end user: IllegalArgumentException(null) instead of the useful description.

In the case that the isValidCompatibilityForFormat does return that it is not valid, it does not tell the user why - Preconditions.checkArgument returns a null IllegalArgumentException, resulting in no description being sent to the user, such as "Compatibility Type must be 'AllowAll' or 'DenyAll' for <user provided serializationFormat>" in the HTTP 400 error.

Once I was able to figure out why I was getting an HTTP 400 error, updating the compatibility type to "AllowAll" fixed my issue in this case, where the following provided me a HTTP 201 return.

{
    "groupName": "mygroup",
    "groupProperties": {
        "serializationFormat": {
            "serializationFormat": "Json"
        },
        "compatibility": {
            "policy": "AllowAny"
        },
        "allowMultipleTypes": true,
        "properties": {}
    }
}

This would be extremely helpful to have documented in the

  • https://github.com/pravega/schema-registry/wiki/REST-API-Usage-Samples
  • https://github.com/pravega/schema-registry/wiki/REST-documentation

I will leave this issue open to associate an improvement PR to these suggestions.

tsteinholz avatar Sep 08 '22 23:09 tsteinholz

@tsteinholz, thank you for the input. We will have a look on this and will work accordingly.

shshashwat avatar Sep 09 '22 03:09 shshashwat