Copilot commented on code in PR #4803:
URL: https://github.com/apache/solr/pull/4803#discussion_r4207239461
##########
solr/webapp/web/js/angular/app.js:
##########
@@ -429,6 +429,9 @@ solrAdminApp.config([
// Schema Designer and Security panels handle errors internally to provide
a better user experience than the global error handler
var isHandledBySchemaDesigner = rejection.config.url &&
rejection.config.url.startsWith("/api/schema-designer/");
var isHandledBySecurity = rejection.config.url &&
rejection.config.url.startsWith("/api/cluster/security/");
+ // HTTP 510 means a feature is switched off in solr.xml, e.g. metrics
collection. The screen
+ // asking for that data explains it in place, so skip the global error
banner.
+ var isDisabledFeature = rejection.status === 510;
Review Comment:
`INVALID_STATE` is Solr's general HTTP 510 code, not a feature-disabled-only
status (for example, `SplitShardCmd` uses it when a parent slice is not active
and `HttpSolrCall` uses it when no active replica is found). Suppressing the
global error handler for every 510 therefore makes unrelated `$http` failures
disappear instead of populating `$rootScope.exceptions`; restrict this check to
the metrics request (or use an endpoint-specific marker).
##########
solr/webapp/web/js/angular/controllers/plugins.js:
##########
@@ -44,6 +45,12 @@ solrAdminApp.controller('PluginsController',
} else {
$scope.plugins = [];
}
+ }, function (response) {
+ // Solr answers HTTP 510 when metrics collection is turned off
in solr.xml
+ $scope.metricsDisabled = response.status === 510;
+ $scope.types = [];
+ $scope.type = null;
+ $scope.plugins = [];
Review Comment:
The new 510 path is not covered by the existing Admin UI tests:
`AdminUiCollectionScreensTest` only exercises the Plugins screen with metrics
enabled, and `AdminUiTestBase` explicitly sets `metricsEnabled` to true. Add a
browser test using a metrics-disabled node/config that asserts the explanatory
message is rendered and no global error banner is recorded; the backend
`MetricsDisabledTest` alone cannot catch regressions in this
controller/template path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
