You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by mike-jumper <gi...@git.apache.org> on 2018/04/27 07:31:46 UTC
[GitHub] guacamole-client pull request #278: GUACAMOLE-526: Handle absolutely all pro...
GitHub user mike-jumper opened a pull request:
https://github.com/apache/guacamole-client/pull/278
GUACAMOLE-526: Handle absolutely all promise rejections.
This change addresses the new "possibly unhandled rejection" error by adding handling for absolutely *all* promise rejections. Each rejection is handled through one of the following mechanisms:
* Ignored via `angular.noop` (if the promise is not REST-related and failure of the promise is expected)
* Ignored via `requestService.IGNORE` (if the promise is REST-related and failure is expected)
* Warning via `requestService.WARN` (if the promise is REST-related and failure of the promise is generally *not* expected, but such failures are better quietly logged to the console than displayed in a modal notification)
* Notification via `guacNotification.SHOW_REQUEST_ERROR` (if the promise is REST-related and any failure should be exposed to the user as a modal notification)
* Handled in some custom fashon via `requestService.createErrorCallback()` (if the promise is REST-related)
The `IGNORE`, `WARN`, and `SHOW_REQUEST_ERROR` constants mentioned above are pre-defined callbacks generated through `requestService.createErrorCallback()`. The `requestService.createErrorCallback()` function produces a new callback function which invokes a given callback *only of the rejection is an instance of the REST `Error` object*. This allows JavaScript failures within the handling of a promise to still result in an error logged to the console.
In addition to the above, I noticed the following issues during my regression testing which had to be addressed:
* A `console.log()` debug statement snuck into the new `toArray` filter.
* An old-style `success()` call (used by the old AngularJS `$http` service) within the password update function.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mike-jumper/guacamole-client fix-angularjs-1.6-warn
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/guacamole-client/pull/278.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #278
----
commit 670ec390b5f33644d854d8bb98e9cff00db381db
Author: Michael Jumper <mj...@...>
Date: 2018-04-24T21:42:18Z
GUACAMOLE-526: Ignore failure to read/write clipboard.
commit 01e19c19dcd9882128f14b2c6286dea3a62878c3
Author: Michael Jumper <mj...@...>
Date: 2018-04-24T21:43:54Z
GUACAMOLE-526: Maintain full correct chain of promises for clipboard read attempts, including the "finally" used for cleanup. The "finally" handler creates a new promise with a potentially unhandled rejection otherwise.
commit 5be303666a226a53e956f8984054993c9b7a2dd4
Author: Michael Jumper <mj...@...>
Date: 2018-04-24T22:22:31Z
GUACAMOLE-526: Remove unnecessary use of $q within authenticationService. Rely on requestService.
commit 6cd07052d8adcca16dc7762aaf54a8a9040016f7
Author: Michael Jumper <mj...@...>
Date: 2018-04-24T22:26:22Z
GUACAMOLE-526: Remove unused injections of $q.
commit bba01bdbc45c74d4b8aafc59223bd16cf2bf5392
Author: Michael Jumper <mj...@...>
Date: 2018-04-24T22:27:18Z
GUACAMOLE-526: Explicitly fail routing if attempting to update the auth token fails.
commit 73eb25f3118d36d4b80f5ac58a01b15b77980e00
Author: Michael Jumper <mj...@...>
Date: 2018-04-24T22:30:58Z
GUACAMOLE-526: Remove unused (and wrong!) getAllActiveConnections() function. Remove now-unnecessary injection of $q.
commit f6f66eec0a9b503b2cb594d6d226cc1ade9dc15c
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T02:03:53Z
GUACAMOLE-526: Ignore failure to load OSK layout.
commit 2d03387b6bb4a44bbf95a903cf98693f7edcb71c
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T02:20:43Z
GUACAMOLE-526: Add missing semicolon.
commit 8c9735d1e7da1a33928b70b4be6659dbf4bc711a
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T03:57:18Z
GUACAMOLE-526: Wrap HTTP response in Error object only if it's an actual HTTP response object.
commit c30b7b0d8030e6b91f102f12df160c63a72b76c5
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T04:11:51Z
GUACAMOLE-526: Add convenience function for generating promise callbacks which handle only REST errors.
commit cc6ade49171627cd89075ab91cde9317196989c2
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T04:19:22Z
GUACAMOLE-526: Add convenience callbacks for ignoring or (quietly) warning of REST errors.
commit 1e5e9b607b56e8cf0fd15b9b58337baa6c0652aa
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T04:20:05Z
GUACAMOLE-526: Add convenience function for displaying modal notifications for REST errors.
commit f6d5e5662b643e0521a3069c14639ca03a9a0714
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T04:20:31Z
GUACAMOLE-526: Add convenience callback for displaying modal notifications strictly for REST errors, while logging all other promise rejections.
commit 266b445c217984fdb59048c09b736098776f6a1f
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T05:15:17Z
GUACAMOLE-526: Handle rejections for absolutely all promises.
commit ae6b5fc8bbc67822308c5aaf038bb2456c6d21f4
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T05:28:18Z
GUACAMOLE-526: Move handling of request error notification to guacNotification, resolving circular dependency.
commit f8bd8d8ec97d2f9d112bafe25b4f5340d5d5cc8c
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T07:02:22Z
GUACAMOLE-526: Remove call to old-style $http success().
commit 2ff980dedebd51e0a8f966f25012cf03563ec8b4
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T07:05:09Z
GUACAMOLE-526: Remove debug console.log() from toArrayFilter().
commit 0d63d6cef442b35e3892305500bffb88b81c16da
Author: Michael Jumper <mj...@...>
Date: 2018-04-27T07:12:36Z
GUACAMOLE-526: Only pull primary connection for sharing profile once identifier is known.
----
---
[GitHub] guacamole-client pull request #278: GUACAMOLE-526: Handle absolutely all pro...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/guacamole-client/pull/278
---
[GitHub] guacamole-client pull request #278: GUACAMOLE-526: Handle absolutely all pro...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/278#discussion_r184893866
--- Diff: guacamole/src/main/webapp/app/rest/services/activeConnectionService.js ---
@@ -66,68 +65,6 @@ angular.module('rest').factory('activeConnectionService', ['$injector',
};
- /**
- * Returns a promise which resolves with all active connections accessible
- * by the current user, as a map of @link{ActiveConnection} maps, as would
- * be returned by getActiveConnections(), grouped by the identifier of
- * their corresponding data source. All given data sources are queried. If
- * an error occurs while retrieving any ActiveConnection map, the promise
- * will be rejected.
- *
- * @param {String[]} dataSources
- * The unique identifier of the data sources containing the active
- * connections to be retrieved. These identifiers correspond to
- * AuthenticationProviders within the Guacamole web application.
- *
- * @param {String[]} [permissionTypes]
- * The set of permissions to filter with. A user must have one or more
- * of these permissions for an active connection to appear in the
- * result. If null, no filtering will be performed. Valid values are
- * listed within PermissionSet.ObjectType.
- *
- * @returns {Promise.<Object.<String, Object.<String, ActiveConnection>>>}
- * A promise which resolves with all active connections available to
- * the current user, as a map of ActiveConnection maps, as would be
- * returned by getActiveConnections(), grouped by the identifier of
- * their corresponding data source.
- */
- service.getAllActiveConnections = function getAllActiveConnections(dataSources, permissionTypes) {
--- End diff --
Not used at all. I'm guessing it must have been added before `dataSourceService` was created, as the only place in the webapp where `getAllActiveConnections()` would have been called is using `dataSourceService` and `getActiveConnections()` instead:
https://github.com/apache/guacamole-client/blob/9a2e0c60879ce0b7dc14870cdb5fe686e2c22eef/guacamole/src/main/webapp/app/settings/directives/guacSettingsSessions.js#L224-L228
---
[GitHub] guacamole-client pull request #278: GUACAMOLE-526: Handle absolutely all pro...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/278#discussion_r184854136
--- Diff: guacamole/src/main/webapp/app/rest/services/activeConnectionService.js ---
@@ -66,68 +65,6 @@ angular.module('rest').factory('activeConnectionService', ['$injector',
};
- /**
- * Returns a promise which resolves with all active connections accessible
- * by the current user, as a map of @link{ActiveConnection} maps, as would
- * be returned by getActiveConnections(), grouped by the identifier of
- * their corresponding data source. All given data sources are queried. If
- * an error occurs while retrieving any ActiveConnection map, the promise
- * will be rejected.
- *
- * @param {String[]} dataSources
- * The unique identifier of the data sources containing the active
- * connections to be retrieved. These identifiers correspond to
- * AuthenticationProviders within the Guacamole web application.
- *
- * @param {String[]} [permissionTypes]
- * The set of permissions to filter with. A user must have one or more
- * of these permissions for an active connection to appear in the
- * result. If null, no filtering will be performed. Valid values are
- * listed within PermissionSet.ObjectType.
- *
- * @returns {Promise.<Object.<String, Object.<String, ActiveConnection>>>}
- * A promise which resolves with all active connections available to
- * the current user, as a map of ActiveConnection maps, as would be
- * returned by getActiveConnections(), grouped by the identifier of
- * their corresponding data source.
- */
- service.getAllActiveConnections = function getAllActiveConnections(dataSources, permissionTypes) {
--- End diff --
Is this method not used?
---