marathon-client icon indicating copy to clipboard operation
marathon-client copied to clipboard

mesosphere.marathon.client.MarathonClient FIX proposal

Open campa opened this issue 6 years ago • 0 comments

The method here try to parse also 401 ( and others ) coded HTTP responses ... this result in a parse problem, but is not.

static class MarathonErrorDecoder implements ErrorDecoder {
		@Override
		public Exception decode(String methodKey, Response response) {
			if (response.status() >= 400 && response.status() < 500 ) {
				try {
					ErrorResponse parsed = ModelUtils.GSON.fromJson(response.body().asReader(), ErrorResponse.class);
					return new MarathonException(response.status(), response.reason(), parsed);
				} catch (IOException e) {
					// intentionally nothing
				}
			}
			return new MarathonException(response.status(), response.reason());
		}
	}

Here my proposal for fixing. Also I suggest to remove the controlling of logging system by System.setEnv from java and so on... It is better to use simple standard java way and control lgging configuraiton using usual and suggested way ( xml or properties conf files ). Using the "env system properties params" way is better to be used by command line and is not reccomended to be forces by java hard coded code. Also is not reccomended to have empty catches ...always

static class MarathonErrorDecoder implements ErrorDecoder {
		@Override
		public Exception decode(String methodKey, Response response) {
			LOG.debug("response {}", response);
			if (response.status() >= 200 && response.status() < 300) {
				try {
					ErrorResponse parsed = ModelUtils.GSON.fromJson(response.body().asReader(), ErrorResponse.class);
					return new MarathonException(response.status(), response.reason(), parsed);
				} catch (IOException e) {
					LOG.warn("decode", e);
				}
			}
			return new MarathonException(response.status(), response.reason());
		}
	}

campa avatar Jul 05 '19 13:07 campa