Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

mesosphere.marathon.client.MarathonClient FIX proposal #18

Open
campa opened this issue Jul 5, 2019 · 0 comments
Open

mesosphere.marathon.client.MarathonClient FIX proposal #18

campa opened this issue Jul 5, 2019 · 0 comments

Comments

@campa
Copy link

campa commented Jul 5, 2019

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());
		}
	}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant