search

spring-cloud / spring-cloud-open-service-broker

Spring Cloud project for creating service brokers that conform to the Open Server Broker API specification
https://spring.io/projects/spring-cloud-open-service-broker
Apache License 2.0
167 stars 118 forks source link

ServiceBrokerInvalidParametersException should return HTTP 400 #262

Closed fang10 closed 4 years ago

fang10 commented 4 years ago

I noticed this file ServiceBrokerBadRequestException was introduced in version 1.x, but it became missing since version 2.x.

What's the reason to drop it?

or should we change ServiceBrokerInvalidParametersException to return 400?

royclarkson commented 4 years ago

The entire project was refactored for version 2. I don't remember why we dropped or changed that exception. Can you provide more information with a scenario or use case that describes the issue you are having? For more information, all of the exceptions and their return values can be seen in the ServiceBrokerExceptionHandler. Thanks.

fang10 commented 4 years ago

Thanks for the info. The reason I asked about it is that we have a need to validate parameter field for provisioning PUT /v2/service_instances/:instance_id. According to the OSB spec https://github.com/openservicebrokerapi/servicebroker/blob/v2.15/spec.md#provisioning, it should return 400 bad request if parameters are not valid.

We can't do it because there is no such exception exists in the library

royclarkson commented 4 years ago

After reviewing the spec, I believe ServiceBrokerInvalidParametersException should be returning a 400, although I wonder if it shouldn't be renamed too. We already have a ServiceBrokerAsyncRequiredException that returns a 422. But we're also missing an exception for the MaintenanceInfoConflict error code that should also return 422. I'll add another issue for that.

royclarkson commented 4 years ago

This is related to #215. You can see there are a few exceptions that the framework throws in relation to invalid parameters that will return a 400. Maybe we still have a gap where the implementing application wants to be able to throw an exception.