New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proxy exception logging fix #33

Merged
merged 4 commits into from Jan 3, 2019

Conversation

Projects
None yet
3 participants
@1000TurquoisePogs
Copy link
Contributor

1000TurquoisePogs commented Nov 27, 2018

Improved logging for what to do when someone forgot to configure a proxy service.

The message format I changed now prints:

[2018-11-27 14:55:10.138 _unp.install INFO] - Setting up proxy (com.zowe.explorer.server.proxy:data) to destination=undefined:undefined/
[2018-11-27 14:55:10.138 _unp.install WARNING] - Exception occurred, plugin (com.zowe.explorer.server.proxy) installation skipped. Message: Proxy (com.zowe.explorer.server.proxy:data) setup failed. Host & Port for proxy destination are required but were missing. For information on how to configure a proxy service, see the Zowe wiki on dataservices (https://github.com/zowe/zlux/wiki/ZLUX-Dataservices)

Instead of

[2018-11-12 18:12:14.040 _unp.install INFO] - com.zowe.explorer.server.proxy: installing static file handlers...
[2018-11-12 18:12:14.041 _unp.install INFO] - Setting up proxy to undefined:undefined/
[2018-11-12 18:12:14.041 _unp.install WARNING] - Error: Proxy: need a host and port
at Object.makeSimpleProxy (C:_Work\bt\zlux\zlux-proxy-server\js\proxy.js:58:11)
at WebApp.makeExternalProxy (C:_Work\bt\zlux\zlux-proxy-server\js\webapp.js:479:25)
at WebApp._installDataServices (C:_Work\bt\zlux\zlux-proxy-server\js\webapp.js:788:47)
at _installDataServices.next ()
at WebApp. (C:_Work\bt\zlux\zlux-proxy-server\js\webapp.js:891:19)
at Generator.next ()
at Generator.tryCatcher (C:_Work\bt\zlux\zlux-proxy-server\js\node_modules\bluebird\js\release\util.js:16:23)
at PromiseSpawn._promiseFulfilled (C:_Work\bt\zlux\zlux-proxy-server\js\node_modules\bluebird\js\release\generators.js:97:49)
at WebApp.installPlugin (C:_Work\bt\zlux\zlux-proxy-server\js\node_modules\bluebird\js\release\generators.js:201:15)
at Server.pluginLoaded (C:_Work\bt\zlux\zlux-proxy-server\js\index.js:320:24)
at PluginLoader.pluginLoader.on.event (C:_Work\bt\zlux\zlux-proxy-server\js\index.js:251:12)
at PluginLoader.emit (events.js:182:13)
at PluginLoader.loadPlugins (C:_Work\bt\zlux\zlux-proxy-server\js\plugin-loader.js:651:12)
at Server. (C:_Work\bt\zlux\zlux-proxy-server\js\index.js:270:23)
at Generator.next ()
at Generator.tryCatcher (C:_Work\bt\zlux\zlux-proxy-server\js\node_modules\bluebird\js\release\util.js:16:23)
at PromiseSpawn._promiseFulfilled (C:_Work\bt\zlux\zlux-proxy-server\js\node_modules\bluebird\js\release\generators.js:97:49)
at Server.start (C:_Work\bt\zlux\zlux-proxy-server\js\node_modules\bluebird\js\release\generators.js:201:15)
at Object. (C:_Work\bt\zlux\zlux-example-server\js\zluxServer.js:18:13)
at Module._compile (internal/modules/cjs/loader.js:688:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
at Module.load (internal/modules/cjs/loader.js:598:32)

NOTE This additionally changes the loading behavior of plugins. Previously, this log was done by throwing an exception, after static content of the plugin had already been configured for use by the server, and afterwards the server still records the plugin as having been installed. I believe that exceptions thrown by dataservices should prevent the plugin from loading further, because something went wrong and so dependencies should not expect this plugin to exist afterward.

Signed-off-by: 1000TurquoisePogs sgrady@rocketsoftware.com

Improved logging for what to do when someone forgot to configure a pr…
…oxy service

Signed-off-by: 1000TurquoisePogs <sgrady@rocketsoftware.com>

@1000TurquoisePogs 1000TurquoisePogs requested a review from zowe/committers-zlux-proxy-server Nov 27, 2018

@wafflebot wafflebot bot added the review label Nov 27, 2018

@rpenny125
Copy link
Contributor

rpenny125 left a comment

I support all the changes, including not loading a plugin whose data service fails to load.

Show resolved Hide resolved js/webapp.js Outdated

timgerstel added a commit to timgerstel/zlux-proxy-server that referenced this pull request Dec 6, 2018

Impleneted zowe#33
Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
Show resolved Hide resolved js/proxy.js Outdated

1000TurquoisePogs added some commits Dec 31, 2018

Minor logging and line length fix
Signed-off-by: 1000TurquoisePogs <sgrady@rocketsoftware.com>
Get rid of unnecessary stack trace for configuration error, improve l…
…ogging output

Signed-off-by: 1000TurquoisePogs <sgrady@rocketsoftware.com>
@1000TurquoisePogs

This comment has been minimized.

Copy link
Contributor Author

1000TurquoisePogs commented Dec 31, 2018

I've fixed it for both of the feedbacks. This should be good now but perhaps you want to give it another quick review.

@1000TurquoisePogs

This comment has been minimized.

Copy link
Contributor Author

1000TurquoisePogs commented Jan 3, 2019

Re-built and tested, this seems good. Merging

@1000TurquoisePogs 1000TurquoisePogs merged commit f0c055c into zowe:master Jan 3, 2019

@wafflebot wafflebot bot removed the review label Jan 3, 2019

@1000TurquoisePogs 1000TurquoisePogs deleted the 1000TurquoisePogs:bugfix/proxy-exception-logging branch Feb 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment