From 078ab15a862d80d42a9e0ce0563cb76ddbe4a721 Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 12 Apr 2021 14:08:49 +0200 Subject: [PATCH] Better error handling and logging --- .../sebserver/gbl/async/CircuitBreaker.java | 4 +- .../remote/webservice/api/RestService.java | 9 ++++ .../DisposedOAuth2RestTemplateException.java | 19 ++++++++ .../OAuth2AuthorizationContextHolder.java | 2 +- .../session/ClientConnectionDetails.java | 11 +++++ .../session/ClientConnectionTable.java | 43 +++++++++++++------ .../servicelayer/dao/impl/ExamDAOImpl.java | 2 +- .../lms/impl/moodle/MoodleCourseAccess.java | 4 +- .../moodle/MoodleRestTemplateFactory.java | 7 ++- 9 files changed, 80 insertions(+), 21 deletions(-) create mode 100644 src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/DisposedOAuth2RestTemplateException.java diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java b/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java index 9a56ffd9..9d214d52 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java @@ -174,7 +174,7 @@ public final class CircuitBreaker { this.state = State.HALF_OPEN; this.failingCount.set(0); return Result.ofError(new RuntimeException( - "Set CircuitBeaker to half-open state. Cause: ", + "Set CircuitBeaker to half-open state. Cause: " + result.getError().getMessage(), result.getError())); } else { // try again @@ -204,7 +204,7 @@ public final class CircuitBreaker { this.state = State.OPEN; return Result.ofError(new RuntimeException( - "Set CircuitBeaker to open state. Cause: ", + "Set CircuitBeaker to open state. Cause: " + result.getError().getMessage(), result.getError())); } else { // on success go to CLOSED state diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestService.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestService.java index 693a267c..355ebedf 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestService.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestService.java @@ -71,4 +71,13 @@ public interface RestService { EntityType entityType, CallType callType); + /** Use this to inject the current SEB Server API access RestTemplate to a long living + * RestCallBuilder. This is usually used to recover from a disposed API access RestTemplate. + * + * @param The generic type of RestCallBuilder + * @param builder the RestCallBuilder to inject the current RestTemplate into. */ + default void injectCurrentRestTemplate(final RestCall.RestCallBuilder builder) { + builder.withRestTemplate(getWebserviceAPIRestTemplate()); + } + } \ No newline at end of file diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/DisposedOAuth2RestTemplateException.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/DisposedOAuth2RestTemplateException.java new file mode 100644 index 00000000..a226f11f --- /dev/null +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/DisposedOAuth2RestTemplateException.java @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2021 ETH Zürich, Educational Development and Technology (LET) + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +package ch.ethz.seb.sebserver.gui.service.remote.webservice.auth; + +public class DisposedOAuth2RestTemplateException extends IllegalStateException { + + private static final long serialVersionUID = -8439656564917103027L; + + public DisposedOAuth2RestTemplateException(final String s) { + super(s); + } + +} diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java index 7d14f697..2fef67f7 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java @@ -132,7 +132,7 @@ public class OAuth2AuthorizationContextHolder implements AuthorizationContextHol if (this.enabled) { return super.doExecute(url, method, requestCallback, responseExtractor); } else { - throw new IllegalStateException( + throw new DisposedOAuth2RestTemplateException( "Error: Forbidden execution call on disabled DisposableOAuth2RestTemplate"); } } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionDetails.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionDetails.java index fa3bb3f3..e09d0194 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionDetails.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionDetails.java @@ -40,6 +40,7 @@ import ch.ethz.seb.sebserver.gui.service.page.PageService; import ch.ethz.seb.sebserver.gui.service.page.event.ActionEvent; import ch.ethz.seb.sebserver.gui.service.page.impl.PageAction; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.RestCall; +import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.DisposedOAuth2RestTemplateException; import ch.ethz.seb.sebserver.gui.service.session.IndicatorData.ThresholdColor; import ch.ethz.seb.sebserver.gui.table.EntityTable; @@ -135,6 +136,7 @@ public class ClientConnectionDetails { .call() .get(error -> { log.error("Unexpected error while trying to get current client connection data: ", error); + recoverFromDisposedRestTemplate(error); return null; }); @@ -234,4 +236,13 @@ public class ClientConnectionDetails { pageContext); } + public void recoverFromDisposedRestTemplate(final Exception error) { + if (log.isDebugEnabled()) { + log.debug("Try to recover from disposed OAuth2 rest template..."); + } + if (error instanceof DisposedOAuth2RestTemplateException) { + this.pageService.getRestService().injectCurrentRestTemplate(this.restCallBuilder); + } + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java index 0f28a867..58a6a1ce 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java @@ -62,6 +62,7 @@ import ch.ethz.seb.sebserver.gui.service.i18n.LocTextKey; import ch.ethz.seb.sebserver.gui.service.page.PageService; import ch.ethz.seb.sebserver.gui.service.page.impl.PageAction; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.RestCall; +import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.DisposedOAuth2RestTemplateException; import ch.ethz.seb.sebserver.gui.service.session.IndicatorData.ThresholdColor; import ch.ethz.seb.sebserver.gui.widget.WidgetFactory; @@ -90,8 +91,7 @@ public final class ClientConnectionTable { private final static LocTextKey CONNECTION_STATUS_TOOLTIP_TEXT_KEY = new LocTextKey("sebserver.monitoring.connection.list.column.status" + Constants.TOOLTIP_TEXT_KEY_SUFFIX); - private final WidgetFactory widgetFactory; - private final ResourceService resourceService; + private final PageService pageService; private final AsyncRunner asyncRunner; private final Exam exam; private final RestCall>.RestCallBuilder restCallBuilder; @@ -124,12 +124,14 @@ public final class ClientConnectionTable { final Collection indicators, final RestCall>.RestCallBuilder restCallBuilder) { - this.widgetFactory = pageService.getWidgetFactory(); - this.resourceService = pageService.getResourceService(); + this.pageService = pageService; this.asyncRunner = asyncRunner; this.exam = exam; this.restCallBuilder = restCallBuilder; + final WidgetFactory widgetFactory = pageService.getWidgetFactory(); + final ResourceService resourceService = pageService.getResourceService(); + final Display display = tableRoot.getDisplay(); this.colorData = new ColorData(display); @@ -143,11 +145,11 @@ public final class ClientConnectionTable { NUMBER_OF_NONE_INDICATOR_COLUMNS); this.localizedClientConnectionStatusNameFunction = - this.resourceService.localizedClientConnectionStatusNameFunction(); + resourceService.localizedClientConnectionStatusNameFunction(); this.statusFilter = EnumSet.noneOf(ConnectionStatus.class); loadStatusFilter(); - this.table = this.widgetFactory.tableLocalized(tableRoot, SWT.MULTI | SWT.V_SCROLL); + this.table = widgetFactory.tableLocalized(tableRoot, SWT.MULTI | SWT.V_SCROLL); final GridLayout gridLayout = new GridLayout(3 + indicators.size(), false); gridLayout.horizontalSpacing = 100; gridLayout.marginWidth = 100; @@ -161,20 +163,20 @@ public final class ClientConnectionTable { this.table.addListener(SWT.Selection, event -> this.notifySelectionChange()); this.table.addListener(SWT.MouseUp, this::notifyTableInfoClick); - this.widgetFactory.tableColumnLocalized( + widgetFactory.tableColumnLocalized( this.table, CONNECTION_ID_TEXT_KEY, CONNECTION_ID_TOOLTIP_TEXT_KEY); - this.widgetFactory.tableColumnLocalized( + widgetFactory.tableColumnLocalized( this.table, CONNECTION_ADDRESS_TEXT_KEY, CONNECTION_ADDRESS_TOOLTIP_TEXT_KEY); - this.widgetFactory.tableColumnLocalized( + widgetFactory.tableColumnLocalized( this.table, CONNECTION_STATUS_TEXT_KEY, CONNECTION_STATUS_TOOLTIP_TEXT_KEY); for (final Indicator indDef : indicators) { - final TableColumn tableColumn = this.widgetFactory.tableColumnLocalized( + final TableColumn tableColumn = widgetFactory.tableColumnLocalized( this.table, new LocTextKey(INDICATOR_NAME_TEXT_KEY_PREFIX + indDef.name), new LocTextKey(INDICATOR_NAME_TEXT_KEY_PREFIX + indDef.type.name)); @@ -187,7 +189,7 @@ public final class ClientConnectionTable { } public WidgetFactory getWidgetFactory() { - return this.widgetFactory; + return this.pageService.getWidgetFactory(); } public boolean isEmpty() { @@ -325,7 +327,11 @@ public final class ClientConnectionTable { this.restCallBuilder .withHeader(API.EXAM_MONITORING_STATE_FILTER, this.statusFilterParam) .call() - .getOrThrow() + .get(error -> { + log.error("Unexpected error while trying to get client connection table data: ", error); + recoverFromDisposedRestTemplate(error); + return Collections.emptyList(); + }) .forEach(data -> { final UpdatableTableItem tableItem = this.tableMapping.computeIfAbsent( data.getConnectionId(), @@ -425,7 +431,7 @@ public final class ClientConnectionTable { private void saveStatusFilter() { try { - this.resourceService + this.pageService .getCurrentUser() .putAttribute( USER_SESSION_STATUS_FILTER_ATTRIBUTE, @@ -440,7 +446,7 @@ public final class ClientConnectionTable { private void loadStatusFilter() { try { - final String attribute = this.resourceService + final String attribute = this.pageService .getCurrentUser() .getAttribute(USER_SESSION_STATUS_FILTER_ATTRIBUTE); this.statusFilter.clear(); @@ -707,4 +713,13 @@ public final class ClientConnectionTable { } + public void recoverFromDisposedRestTemplate(final Exception error) { + if (log.isDebugEnabled()) { + log.debug("Try to recover from disposed OAuth2 rest template..."); + } + if (error instanceof DisposedOAuth2RestTemplateException) { + this.pageService.getRestService().injectCurrentRestTemplate(this.restCallBuilder); + } + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java index 9f9f355c..fe65a4a7 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java @@ -941,7 +941,7 @@ public class ExamDAOImpl implements ExamDAO { } } } catch (final Exception e) { - log.warn("Failed to try to recover from Moodle quiz restore: ", e.getMessage()); + log.warn("Failed to try to recover from Moodle quiz restore: {}", e.getMessage()); } return null; } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java index 2d192400..375557ac 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java @@ -448,14 +448,14 @@ public class MoodleCourseAccess extends CourseAccess { if (courses == null) { log.error("No courses found for ids: {} on LMS: {}", ids, this.lmsSetup.name); - Collections.emptyList(); + return Collections.emptyList(); } logMoodleWarnings(courses.warnings); if (courses.courses == null || courses.courses.isEmpty()) { log.error("No courses found for ids: {} on LMS: {}", ids, this.lmsSetup.name); - Collections.emptyList(); + return Collections.emptyList(); } return courses.courses; diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java index 522452d9..65819f26 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java @@ -324,9 +324,14 @@ class MoodleRestTemplateFactory { } final String body = response.getBody(); + // NOTE: for some unknown reason, Moodles API error responses come with a 200 OK response HTTP Status // So this is a special Moodle specific error handling here... - if (body.startsWith("{exception")) { + if (body.startsWith("{exception") || body.contains("\"exception\":")) { + // Reset access token to get new on next call (fix access if token is expired) + // TODO find a way to verify token invalidity response from Moodle. + // Unfortunately there is not a lot of Moodle documentation for the API error handling around. + this.accessToken = null; throw new RuntimeException( "Failed to call Moodle webservice API function: " + functionName + " lms setup: " + MoodleRestTemplateFactory.this.lmsSetup + " response: " + body);