diff --git a/src/config.c b/src/config.c index 1827abbae3..0bf8df196c 100644 --- a/src/config.c +++ b/src/config.c @@ -674,6 +674,8 @@ static void restoreBackupConfig(standardConfig **set_configs, sds *old_values, i void configSetCommand(client *c) { const char *errstr = NULL; + const char *invalid_arg_name = NULL; + const char *err_arg_name = NULL; standardConfig **set_configs; /* TODO: make this a dict for better performance */ sds *new_values; sds *old_values = NULL; @@ -712,6 +714,7 @@ void configSetCommand(client *c) { if (config->flags & IMMUTABLE_CONFIG) { /* Note: we don't abort the loop since we still want to handle redacting sensitive configs (above) */ errstr = "can't set immutable config"; + err_arg_name = c->argv[2+i*2]->ptr; invalid_args = 1; } @@ -720,6 +723,7 @@ void configSetCommand(client *c) { if (set_configs[j] == config) { /* Note: we don't abort the loop since we still want to handle redacting sensitive configs (above) */ errstr = "duplicate parameter"; + err_arg_name = c->argv[2+i*2]->ptr; invalid_args = 1; break; } @@ -733,7 +737,7 @@ void configSetCommand(client *c) { /* Fail if we couldn't find this config */ /* Note: we don't abort the loop since we still want to handle redacting sensitive configs (above) */ if (!invalid_args && !set_configs[i]) { - errstr = "unrecognized parameter"; + invalid_arg_name = c->argv[2+i*2]->ptr; invalid_args = 1; } } @@ -749,6 +753,7 @@ void configSetCommand(client *c) { int res = performInterfaceSet(set_configs[i], new_values[i], &errstr); if (!res) { restoreBackupConfig(set_configs, old_values, i+1, NULL); + err_arg_name = set_configs[i]->name; goto err; } else if (res == 1) { /* A new value was set, if this config has an apply function then store it for execution later */ @@ -775,6 +780,7 @@ void configSetCommand(client *c) { if (!apply_fns[i](&errstr)) { serverLog(LL_WARNING, "Failed applying new configuration. Possibly related to new %s setting. Restoring previous settings.", set_configs[config_map_fns[i]]->name); restoreBackupConfig(set_configs, old_values, config_count, apply_fns); + err_arg_name = set_configs[config_map_fns[i]]->name; goto err; } } @@ -782,10 +788,12 @@ void configSetCommand(client *c) { goto end; err: - if (errstr) { - addReplyErrorFormat(c,"Config set failed - %s", errstr); + if (invalid_arg_name) { + addReplyErrorFormat(c,"Unknown option or number of arguments for CONFIG SET - '%s'", invalid_arg_name); + } else if (errstr) { + addReplyErrorFormat(c,"CONFIG SET failed (possibly related to argument '%s') - %s", err_arg_name, errstr); } else { - addReplyError(c,"Invalid arguments"); + addReplyErrorFormat(c,"CONFIG SET failed (possibly related to argument '%s')", err_arg_name); } end: zfree(set_configs); diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index cf2b0e39e5..5e4d2b5e3a 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -322,7 +322,7 @@ start_server {tags {"introspection"}} { # Set some value to maxmemory assert_equal [r config set maxmemory 10000002] "OK" # Set another value to maxmeory together with another invalid config - assert_error "ERR Config set failed - percentage argument must be less or equal to 100" { + assert_error "ERR CONFIG SET failed (possibly related to argument 'maxmemory-clients') - percentage argument must be less or equal to 100" { r config set maxmemory 10000001 maxmemory-clients 200% client-query-buffer-limit invalid } # Validate we rolled back to original values @@ -375,7 +375,7 @@ start_server {tags {"introspection"}} { # Try to listen on the used port, pass some more configs to make sure the # returned failure message is for the first bad config and everything is rolled back. - assert_error "ERR Config set failed - Unable to listen on this port*" { + assert_error "ERR CONFIG SET failed (possibly related to argument 'port') - Unable to listen on this port*" { eval "r config set $some_configs" }