Server error (500) returned after editing Catalog item with used name

[rvsia]

Services > Catalogs > Catalogs > Configuration > New/Edit

Description

• When you add a new Catalog with used name it returns 400 error
• However when you edit some Catalog and change its name to used name it returns 500 error

http://localhost:3000/api/service_catalogs/45

body

{
  "action": "edit",
  "resource": {
    "name": "Aa" // something what is already used
  }
}

response

500 Interval Server Error

{
"error": {
   "kind": "internal_server_error",
    "message": "Validation failed: ServiceTemplateCatalog: Name has already been taken",
    "klass": "ActiveRecord::RecordInvalid"
    }
}

[himdel]

API Request:    {:requested_at=>"2019-04-12 14:07:20 UTC", :method=>"POST", :url=>"http://localhost:3000/api/service_catalogs/1"}
Authentication: {:type=>"basic", :token=>nil, :x_miq_group=>nil, :user=>"admin"}
Authorization:  {:user=>"admin", :group=>"EvmGroup-super_administrator", :role=>"EvmRole-super_administrator", :tenant=>"My Company"}
Request:        {:method=>:post, :action=>"edit", :fullpath=>"/api/service_catalogs/1", :url=>"http://localhost:3000/api/service_catalogs/1", :base=>"http://localhost:3000", :path=>"/api/service_catalogs/1", :prefix=>"/api", :version=>"4.1.0-pre", :api_prefix=>"http://localhost:3000/api", :collection=>"service_catalogs", :c_suffix=>nil, :collection_id=>"1", :subcollection=>nil, :subcollection_id=>nil}
Parameters:     {"action"=>"update", "controller"=>"api/service_catalogs", "format"=>"json", "body"=>{"action"=>"edit", "resource"=>{"name"=>"Single Service"}}}

ActiveRecord::RecordInvalid: Validation failed: ServiceTemplateCatalog: Name has already been taken

activerecord-5.0.7.2/lib/active_record/validations.rb:78:in `raise_validation_error'
activerecord-5.0.7.2/lib/active_record/validations.rb:50:in `save!'
activerecord-5.0.7.2/lib/active_record/attribute_methods/dirty.rb:30:in `save!'
activerecord-5.0.7.2/lib/active_record/transactions.rb:324:in `block in save!'
activerecord-5.0.7.2/lib/active_record/transactions.rb:395:in `block in with_transaction_returning_status'
activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/database_statements.rb:230:in `transaction'
activerecord-5.0.7.2/lib/active_record/transactions.rb:211:in `transaction'
activerecord-5.0.7.2/lib/active_record/transactions.rb:392:in `with_transaction_returning_status'
activerecord-5.0.7.2/lib/active_record/transactions.rb:324:in `save!'
activerecord-5.0.7.2/lib/active_record/suppressor.rb:45:in `save!'
activerecord-5.0.7.2/lib/active_record/persistence.rb:288:in `block in update!'
activerecord-5.0.7.2/lib/active_record/transactions.rb:395:in `block in with_transaction_returning_status'
activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/database_statements.rb:232:in `block in transaction'
activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/transaction.rb:189:in `within_new_transaction'
activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/database_statements.rb:232:in `transaction'
activerecord-5.0.7.2/lib/active_record/transactions.rb:211:in `transaction'
activerecord-5.0.7.2/lib/active_record/transactions.rb:392:in `with_transaction_returning_status'
activerecord-5.0.7.2/lib/active_record/persistence.rb:286:in `update!'
manageiq-api-9240a6cb1e59/app/controllers/api/base_controller/generic.rb:53:in `edit_resource'
manageiq-api-9240a6cb1e59/app/controllers/api/base_controller/manager.rb:105:in `update_one_collection'
manageiq-api-9240a6cb1e59/app/controllers/api/base_controller/manager.rb:92:in `get_and_update_one_collection'
manageiq-api-9240a6cb1e59/app/controllers/api/base_controller/manager.rb:15:in `update_collection'
manageiq-api-9240a6cb1e59/app/controllers/api/base_controller.rb:94:in `update'
actionpack-5.0.7.2/lib/action_controller/metal/basic_implicit_render.rb:4:in `send_action'
actionpack-5.0.7.2/lib/abstract_controller/base.rb:188:in `process_action'
actionpack-5.0.7.2/lib/action_controller/metal/rendering.rb:30:in `process_action'
actionpack-5.0.7.2/lib/abstract_controller/callbacks.rb:20:in `block in process_action'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:126:in `call'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:506:in `block (2 levels) in compile'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:455:in `call'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:101:in `__run_callbacks__'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:750:in `_run_process_action_callbacks'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:90:in `run_callbacks'
actionpack-5.0.7.2/lib/abstract_controller/callbacks.rb:19:in `process_action'
actionpack-5.0.7.2/lib/action_controller/metal/rescue.rb:20:in `process_action'
actionpack-5.0.7.2/lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
activesupport-5.0.7.2/lib/active_support/notifications.rb:164:in `block in instrument'
activesupport-5.0.7.2/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
activesupport-5.0.7.2/lib/active_support/notifications.rb:164:in `instrument'
actionpack-5.0.7.2/lib/action_controller/metal/instrumentation.rb:30:in `process_action'
actionpack-5.0.7.2/lib/action_controller/metal/params_wrapper.rb:248:in `process_action'
activerecord-5.0.7.2/lib/active_record/railties/controller_runtime.rb:18:in `process_action'
actionpack-5.0.7.2/lib/abstract_controller/base.rb:126:in `process'
actionpack-5.0.7.2/lib/action_controller/metal.rb:190:in `dispatch'
actionpack-5.0.7.2/lib/action_controller/metal.rb:262:in `dispatch'
actionpack-5.0.7.2/lib/action_dispatch/routing/route_set.rb:50:in `dispatch'
actionpack-5.0.7.2/lib/action_dispatch/routing/route_set.rb:32:in `serve'
actionpack-5.0.7.2/lib/action_dispatch/journey/router.rb:39:in `block in serve'
actionpack-5.0.7.2/lib/action_dispatch/journey/router.rb:26:in `each'
actionpack-5.0.7.2/lib/action_dispatch/journey/router.rb:26:in `serve'
actionpack-5.0.7.2/lib/action_dispatch/routing/route_set.rb:727:in `call'
manageiq-graphql-38bd898e7eb3/lib/manageiq/graphql/rest_api_proxy.rb:18:in `call'
secure_headers-3.0.3/lib/secure_headers/middleware.rb:10:in `call'
manageiq/lib/request_started_on_middleware.rb:12:in `call'
rack-2.0.7/lib/rack/etag.rb:25:in `call'
rack-2.0.7/lib/rack/conditional_get.rb:38:in `call'
rack-2.0.7/lib/rack/head.rb:12:in `call'
rack-2.0.7/lib/rack/session/abstract/id.rb:232:in `context'
rack-2.0.7/lib/rack/session/abstract/id.rb:226:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/cookies.rb:613:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/callbacks.rb:38:in `block in call'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:97:in `__run_callbacks__'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:750:in `_run_call_callbacks'
activesupport-5.0.7.2/lib/active_support/callbacks.rb:90:in `run_callbacks'
actionpack-5.0.7.2/lib/action_dispatch/middleware/callbacks.rb:36:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/executor.rb:12:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/debug_exceptions.rb:49:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
railties-5.0.7.2/lib/rails/rack/logger.rb:36:in `call_app'
railties-5.0.7.2/lib/rails/rack/logger.rb:26:in `call'
sprockets-rails-3.2.1/lib/sprockets/rails/quiet_assets.rb:13:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/request_id.rb:24:in `call'
rack-2.0.7/lib/rack/method_override.rb:22:in `call'
rack-2.0.7/lib/rack/runtime.rb:22:in `call'
activesupport-5.0.7.2/lib/active_support/cache/strategy/local_cache_middleware.rb:28:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/executor.rb:12:in `call'
actionpack-5.0.7.2/lib/action_dispatch/middleware/static.rb:136:in `call'
rack-2.0.7/lib/rack/sendfile.rb:111:in `call'
railties-5.0.7.2/lib/rails/engine.rb:522:in `call'
puma-3.7.1/lib/puma/configuration.rb:232:in `call'
puma-3.7.1/lib/puma/server.rb:578:in `handle_request'
puma-3.7.1/lib/puma/server.rb:415:in `process_client'
puma-3.7.1/lib/puma/server.rb:275:in `block in run'
puma-3.7.1/lib/puma/thread_pool.rb:120:in `block in spawn_thread'

Response:       {:completed_at=>"2019-04-12 14:07:20 UTC", :size=>"0.163 KBytes", :time_taken=>"0.144 Seconds", :status=>500}

(from api.log)

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

[h-kataria]

was not able to recreate issue in UI, seems like this issue was addressed in UI with form conversion to react in ManageIQ/manageiq-ui-classic#5139

@martinpovolny do we need to fix something on API side for this?

[h-kataria]

@miq-bot assign @martinpovolny

[martinpovolny]

This is consistent with other places. For example:

lucifer ruby-2.5.5 - [~/Projects/manageiq-api] (master)$ echo '{ "action": "edit", "resource": { "name": "Loic" } }' | api post roles/20

{
  "error": {
    "kind": "internal_server_error",
    "message": "Validation failed: MiqUserRole: Name has already been taken",
    "klass": "ActiveRecord::RecordInvalid"
  }
}

We probably should have a better generic way of handling this.

I am not sure, but maybe the right HTTP code to return would be "422 Unprocessable Entity".

Also the validation error should probably be better structured so that the UI can present it to the user and possibly a translation in the current locale shoud be returned.

Thoughts on this one? @rvsia, @himdel, @abellotti ?

[himdel]

👍, but looks like we're using "400 Bad Request" in other places.

As for i18n vs the error message, maybe we should use a similar mechanism as notifications (miqFormatNotification) - serve a translatable format string and the parameters separately?
(Not sure if the API should be able to tell the current language.)

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation.