Sending an empty note in diff results in 500 error
Summary
Sending an empty note in a diff results in a 500 error. An empty note is not normally possible via the UI but if you use the keyboard shortcut (CTRL+Enter/CMD+Enter), it's possible to send an empty note.
Steps to reproduce
- Create a new comment on an MR's diff
- CTRL+Enter/CMD+Enter to start the review
- Submit review
Example Project
What is the current bug behavior?
Empty comment throws 500 error.
What is the expected correct behavior?
Empty comment either cannot be sent and/or backend inserts into the DB but with an empty string value rather than a NULL
.
Relevant logs and/or screenshots
The contents of the POST to GitLab.com has an empty string:
{"noteable_type":"MergeRequest","noteable_id":269987583,"note":"","approve":false,"approval_password":"","reviewer_state":"reviewed"}
Error reported by GitLab.com
```json { "action": "publish", "cf_ray": "8375b695d8361a02-KIX", "component": "gitlab", "controller": "Projects::MergeRequests::DraftsController", "correlation_id": "01HHXXEPGFTP0CQHPT0M92JFR0", "cpu_s": 0.326046, "db_cached_count": 7, "db_ci_cached_count": 0, "db_ci_count": 0, "db_ci_duration_s": 0, "db_ci_replica_cached_count": 0, "db_ci_replica_count": 0, "db_ci_replica_duration_s": 0, "db_ci_replica_wal_cached_count": 0, "db_ci_replica_wal_count": 0, "db_ci_wal_cached_count": 0, "db_ci_wal_count": 0, "db_count": 29, "db_duration_s": 0.03498, "db_embedding_cached_count": 0, "db_embedding_count": 0, "db_embedding_duration_s": 0, "db_embedding_replica_cached_count": 0, "db_embedding_replica_count": 0, "db_embedding_replica_duration_s": 0, "db_embedding_replica_wal_cached_count": 0, "db_embedding_replica_wal_count": 0, "db_embedding_wal_cached_count": 0, "db_embedding_wal_count": 0, "db_main_cached_count": 2, "db_main_count": 13, "db_main_duration_s": 0.016, "db_main_replica_cached_count": 5, "db_main_replica_count": 16, "db_main_replica_duration_s": 0.01, "db_main_replica_wal_cached_count": 0, "db_main_replica_wal_count": 0, "db_main_wal_cached_count": 0, "db_main_wal_count": 0, "db_primary_cached_count": 2, "db_primary_count": 13, "db_primary_duration_s": 0.016, "db_primary_wal_cached_count": 0, "db_primary_wal_count": 0, "db_replica_cached_count": 5, "db_replica_count": 16, "db_replica_duration_s": 0.01, "db_replica_wal_cached_count": 0, "db_replica_wal_count": 0, "db_write_count": 2, "duration_s": 0.37798, "environment": "gprd", "exception.backtrace": [ "lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `public_send'", "lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `block in write_using_load_balancer'", "lib/gitlab/database/load_balancing/load_balancer.rb:141:in `block in read_write'", "lib/gitlab/database/load_balancing/load_balancer.rb:235:in `retry_with_backoff'", "lib/gitlab/database/load_balancing/load_balancer.rb:130:in `read_write'", "lib/gitlab/database/load_balancing/connection_proxy.rb:126:in `write_using_load_balancer'", "lib/gitlab/database/load_balancing/connection_proxy.rb:61:in `block (2 levels) in '", "app/models/diff_note_position.rb:40:in `create_or_update_for'", "app/services/discussions/capture_diff_note_position_service.rb:27:in `execute'", "app/services/draft_notes/publish_service.rb:99:in `block in capture_diff_note_positions'", "app/services/draft_notes/publish_service.rb:98:in `each'", "app/services/draft_notes/publish_service.rb:98:in `capture_diff_note_positions'", "app/services/draft_notes/publish_service.rb:45:in `publish_draft_notes'", "app/services/draft_notes/publish_service.rb:11:in `execute'", "app/controllers/projects/merge_requests/drafts_controller.rb:56:in `publish'", "ee/lib/gitlab/ip_address_state.rb:10:in `with'", "ee/app/controllers/ee/application_controller.rb:45:in `set_current_ip_address'", "app/controllers/application_controller.rb:470:in `set_current_admin'", "lib/gitlab/session.rb:11:in `with_session'", "app/controllers/application_controller.rb:461:in `set_session_storage'", "lib/gitlab/i18n.rb:114:in `with_locale'", "lib/gitlab/i18n.rb:120:in `with_user_locale'", "app/controllers/application_controller.rb:452:in `set_locale'", "app/controllers/application_controller.rb:445:in `set_current_context'", "ee/lib/omni_auth/strategies/group_saml.rb:41:in `other_phase'", "lib/gitlab/metrics/elasticsearch_rack_middleware.rb:16:in `call'", "lib/gitlab/middleware/memory_report.rb:13:in `call'", "lib/gitlab/middleware/speedscope.rb:13:in `call'", "lib/gitlab/database/load_balancing/rack_middleware.rb:23:in `call'", "lib/gitlab/middleware/rails_queue_duration.rb:33:in `call'", "lib/gitlab/etag_caching/middleware.rb:21:in `call'", "lib/gitlab/metrics/rack_middleware.rb:16:in `block in call'", "lib/gitlab/metrics/web_transaction.rb:46:in `run'", "lib/gitlab/metrics/rack_middleware.rb:16:in `call'", "lib/gitlab/middleware/go.rb:20:in `call'", "lib/gitlab/middleware/query_analyzer.rb:11:in `block in call'", "lib/gitlab/database/query_analyzer.rb:37:in `within'", "lib/gitlab/middleware/query_analyzer.rb:11:in `call'", "lib/gitlab/middleware/multipart.rb:173:in `call'", "lib/gitlab/middleware/read_only/controller.rb:50:in `call'", "lib/gitlab/middleware/read_only.rb:18:in `call'", "lib/gitlab/middleware/same_site_cookies.rb:27:in `call'", "lib/gitlab/middleware/path_traversal_check.rb:35:in `call'", "lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call'", "lib/gitlab/middleware/basic_health_check.rb:25:in `call'", "lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call'", "lib/gitlab/middleware/request_context.rb:15:in `call'", "lib/gitlab/middleware/webhook_recursion_detection.rb:15:in `call'", "config/initializers/fix_local_cache_middleware.rb:11:in `call'", "lib/gitlab/middleware/compressed_json.rb:44:in `call'", "lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:19:in `call'", "lib/gitlab/middleware/sidekiq_web_static.rb:20:in `call'", "lib/gitlab/metrics/requests_rack_middleware.rb:79:in `call'", "lib/gitlab/middleware/release_env.rb:13:in `call'" ], "exception.cause_class": "PG::NotNullViolation", "exception.class": "ActiveRecord::NotNullViolation", "exception.message": "PG::NotNullViolation: ERROR: null value in column \"note_id\" of relation \"diff_note_positions\" violates not-null constraint\nDETAIL: Failing row contains (227827481, null, null, 13, 0, 0, 8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_13_13, \\x33343036666136303236633461356137393637356630313730306537343537..., \\x33343036666136303236633461356137393637356630313730306537343537..., \\x39383531366231643764613165653433343430303534373831343039373131..., README.md, README.md).\n", "exception.sql": "/*application:web,correlation_id:01HHXXEPGFTP0CQHPT0M92JFR0,endpoint_id:Projects::MergeRequests::DraftsController#publish,db_config_name:main*/ INSERT INTO \"diff_note_positions\" (\"base_sha\",\"start_sha\",\"head_sha\",\"old_path\",\"new_path\",\"old_line\",\"new_line\",\"diff_content_type\",\"diff_type\",\"line_code\",\"note_id\") VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) ON CONFLICT (\"note_id\",\"diff_type\") DO UPDATE SET \"base_sha\"=excluded.\"base_sha\",\"start_sha\"=excluded.\"start_sha\",\"head_sha\"=excluded.\"head_sha\",\"old_path\"=excluded.\"old_path\",\"new_path\"=excluded.\"new_path\",\"old_line\"=excluded.\"old_line\",\"new_line\"=excluded.\"new_line\",\"diff_content_type\"=excluded.\"diff_content_type\",\"line_code\"=excluded.\"line_code\" RETURNING \"id\"", "format": "json", "gitaly_calls": 13, "gitaly_duration_s": 0.164586, "logtag": "F", "mem_bytes": 9910664, "mem_mallocs": 44941, "mem_objects": 131490, "mem_total_bytes": 15170264, "meta.caller_id": "Projects::MergeRequests::DraftsController#publish", "meta.client_id": "user/5749275", "meta.feature_category": "code_review_workflow", "meta.project": "mbadeau/zd-477403", "meta.remote_ip": "", "meta.root_namespace": "mbadeau", "meta.user": "mbadeau", "meta.user_id": 5749275, "method": "POST", "params": [ { "key": "noteable_type", "value": "MergeRequest" }, { "key": "noteable_id", "value": "269987583" }, { "key": "note", "value": "[FILTERED]" }, { "key": "approve", "value": "false" }, { "key": "approval_password", "value": "[FILTERED]" }, { "key": "reviewer_state", "value": "reviewed" }, { "key": "namespace_id", "value": "mbadeau" }, { "key": "project_id", "value": "zd-477403" }, { "key": "merge_request_id", "value": "1" }, { "key": "draft", "value": "{\"noteable_type\"=>\"MergeRequest\", \"noteable_id\"=>269987583, \"note\"=>\"[FILTERED]\", \"approve\"=>false, \"approval_password\"=>\"[FILTERED]\", \"reviewer_state\"=>\"reviewed\"}" } ], "path": "/mbadeau/zd-477403/-/merge_requests/1/drafts/publish", "pid": 71, "queue_duration_s": 0.022256, "rate_limiting_gates": [], "redis_allowed_cross_slot_calls": 1, "redis_calls": 13, "redis_cluster_shared_state_calls": 1, "redis_cluster_shared_state_duration_s": 0.000478, "redis_cluster_shared_state_read_bytes": 2, "redis_cluster_shared_state_write_bytes": 94, "redis_db_load_balancing_calls": 3, "redis_db_load_balancing_duration_s": 0.000999, "redis_db_load_balancing_write_bytes": 180, "redis_duration_s": 0.005646, "redis_feature_flag_calls": 1, "redis_feature_flag_duration_s": 0.000617, "redis_feature_flag_read_bytes": 199, "redis_feature_flag_write_bytes": 67, "redis_queues_calls": 2, "redis_queues_duration_s": 0.000523, "redis_queues_metadata_calls": 1, "redis_queues_metadata_duration_s": 0.000868, "redis_queues_metadata_read_bytes": 2, "redis_queues_metadata_write_bytes": 208, "redis_queues_read_bytes": 6, "redis_queues_write_bytes": 840, "redis_rate_limiting_calls": 1, "redis_rate_limiting_duration_s": 0.000555, "redis_rate_limiting_read_bytes": 2, "redis_rate_limiting_write_bytes": 78, "redis_read_bytes": 1016, "redis_repository_cache_calls": 1, "redis_repository_cache_duration_s": 0.000467, "redis_repository_cache_read_bytes": 102, "redis_repository_cache_write_bytes": 50, "redis_sessions_allowed_cross_slot_calls": 1, "redis_sessions_calls": 3, "redis_sessions_duration_s": 0.001139, "redis_sessions_read_bytes": 703, "redis_sessions_write_bytes": 628, "redis_write_bytes": 2145, "remote_ip": "", "request_urgency": "low", "shard": "default", "stage": "main", "status": 500, "subcomponent": "production_json", "target_duration_s": 5, "tier": "sv", "time": "2023-12-18T07:32:40.496Z", "type": "web", "ua": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:120.0) Gecko/20100101 Firefox/120.0", "user_id": 5749275, "username": "mbadeau", "view_duration_s": 0, "worker_id": "puma_2" } ```Output of checks
This bug happens on GitLab.com
Results of GitLab environment info
Expand for output related to GitLab environment info
(For installations with omnibus-gitlab package run and paste the output of: `sudo gitlab-rake gitlab:env:info`) (For installations from source run and paste the output of: `sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)
Results of GitLab application Check
Expand for output related to the GitLab application check
(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true
)(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true
)(we will only investigate if the tests are passing)