[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15150151#comment-15150151
 ] 

ASF subversion and git services commented on CB-10518:
--

Commit b7920505e56e4371b868bc5f0a3f852d4249fe31 in cordova-lib's branch 
refs/heads/master from [~vladimir.kotikov]
[ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=b792050 ]

CB-10518 Correct log level and error messages for some cordova errors

This closes #383


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15150153#comment-15150153
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user asfgit closed the pull request at:

https://github.com/apache/cordova-lib/pull/383


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15149069#comment-15149069
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r53056714
  
--- Diff: cordova-lib/src/cordova/compile.js ---
@@ -40,7 +40,7 @@ module.exports = function compile(options) {
 }).then(function() {
 return hooksRunner.fire('after_compile', options);
 }, function(error) {
-events.emit('log', 'ERROR building one of the platforms: ' + error 
+ '\nYou may not have the required environment or OS to build this project');
+events.emit('warn', 'ERROR building one of the platforms: ' + 
error + '\nYou may not have the required environment or OS to build this 
project');
--- End diff --

Let's remove the warning if it's not needed and looks like other commands 
do not log anyway.


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15145078#comment-15145078
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r52782786
  
--- Diff: cordova-lib/src/cordova/compile.js ---
@@ -40,7 +40,7 @@ module.exports = function compile(options) {
 }).then(function() {
 return hooksRunner.fire('after_compile', options);
 }, function(error) {
-events.emit('log', 'ERROR building one of the platforms: ' + error 
+ '\nYou may not have the required environment or OS to build this project');
+events.emit('warn', 'ERROR building one of the platforms: ' + 
error + '\nYou may not have the required environment or OS to build this 
project');
--- End diff --

Also when should one use `events.emit('error', ...)` For all errors now we 
are using warning - this is confusing.


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15145066#comment-15145066
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r52782123
  
--- Diff: cordova-lib/src/cordova/create.js ---
@@ -191,8 +191,8 @@ function create(dir, optionalId, optionalName, cfg) {
 
 return remoteLoad.gitClone(gitURL, branch).fail(
 function(err) {
-events.emit('verbose', err);
-return Q.reject('Failed to retrieve '+ 
cfg.lib.www.url + ' using git.');
+events.emit('warn', err.message);
+return Q.reject(new CordovaError('Failed to 
retrieve '+ cfg.lib.www.url + ' using git.'));
--- End diff --

Should the `err.message` be appended to new CordovaError or the original 
`err` be passed as inner error?


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15145072#comment-15145072
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r52782454
  
--- Diff: cordova-lib/src/cordova/compile.js ---
@@ -40,7 +40,7 @@ module.exports = function compile(options) {
 }).then(function() {
 return hooksRunner.fire('after_compile', options);
 }, function(error) {
-events.emit('log', 'ERROR building one of the platforms: ' + error 
+ '\nYou may not have the required environment or OS to build this project');
+events.emit('warn', 'ERROR building one of the platforms: ' + 
error + '\nYou may not have the required environment or OS to build this 
project');
--- End diff --

So what's the guidance for error handling? We should emit a warning with 
detailed error message and then reject the promise with the actual error? I 
notice below you sometimes create a CordovaError to reject the promise. It will 
be great if we can come up with consistent set of guidelines for this.


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15145075#comment-15145075
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r52782573
  
--- Diff: cordova-lib/src/cordova/targets.js ---
@@ -25,9 +25,9 @@ var cordova_util = require('./util'),
 
 function handleError(error) {
 if (error.code === 'ENOENT') {
-events.emit('log', 'Platform does not support ' + this.script);
+events.emit('warn', 'Platform does not support ' + this.script);
 } else {
-events.emit('log', 'An unexpected error has occured');
+events.emit('warn', 'An unexpected error has occured while running 
' + this.script);
--- End diff --

Should we log the error code? It's often useful to have that.


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15145238#comment-15145238
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r52794358
  
--- Diff: cordova-lib/src/cordova/targets.js ---
@@ -25,9 +25,9 @@ var cordova_util = require('./util'),
 
 function handleError(error) {
 if (error.code === 'ENOENT') {
-events.emit('log', 'Platform does not support ' + this.script);
+events.emit('warn', 'Platform does not support ' + this.script);
 } else {
-events.emit('log', 'An unexpected error has occured');
+events.emit('warn', 'An unexpected error has occured while running 
' + this.script);
--- End diff --

Agree. I'll update this


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15145236#comment-15145236
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r52794281
  
--- Diff: cordova-lib/src/cordova/run.js ---
@@ -44,7 +44,7 @@ module.exports = function run(options) {
 }).then(function() {
 return hooksRunner.fire('after_run', options);
 }, function(error) {
-events.emit('log', 'ERROR running one or more of the platforms: ' 
+ error + '\nYou may not have the required environment or OS to run this 
project');
+events.emit('warn', 'ERROR running one or more of the platforms: ' 
+ error + '\nYou may not have the required environment or OS to run this 
project');
--- End diff --

As in the first comment I'm not sure if we should emit this here because it 
is not consistent and not very informative. Maybe rethrow with this message. 
@nikhilkh, what do you think?


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15145232#comment-15145232
 ] 

ASF GitHub Bot commented on CB-10518:
-

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/383#discussion_r52793854
  
--- Diff: cordova-lib/src/cordova/compile.js ---
@@ -40,7 +40,7 @@ module.exports = function compile(options) {
 }).then(function() {
 return hooksRunner.fire('after_compile', options);
 }, function(error) {
-events.emit('log', 'ERROR building one of the platforms: ' + error 
+ '\nYou may not have the required environment or OS to build this project');
+events.emit('warn', 'ERROR building one of the platforms: ' + 
error + '\nYou may not have the required environment or OS to build this 
project');
--- End diff --

I'm not sure if we should continue emitting message here at all, because it 
is not consistent with other similar places, e.g. but `build` and `emulate` do 
not emit this message.  Also the message is not very informative, as `compile` 
might fail due to a lot of different reasons.

In general for cases like this, when initial error message might not be 
very descriptive, i'd rather reject a promise with (or throw in case of sync 
code) a new `CordovaError` with appropriate message and original error object, 
attached.


> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

[jira] [Commented] (CB-10518) Cordova reports error events with incorrect log level

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CB-10518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15140616#comment-15140616
 ] 

ASF GitHub Bot commented on CB-10518:
-

GitHub user vladimir-kotikov opened a pull request:

https://github.com/apache/cordova-lib/pull/383

CB-10518 Correct log level and error messages for some cordova errors

Issue [CB-10518](https://issues.apache.org/jira/browse/CB-10518)

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/MSOpenTech/cordova-lib CB-10518

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-lib/pull/383.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 #383


commit 6ab2d67aa6d428c8f0214e75c43974a5cd483a7b
Author: Vladimir Kotikov 
Date:   2016-02-10T10:44:46Z

CB-10518 Correct log level and error messages for some cordova errors




> Cordova reports error events with incorrect log level
> -
>
> Key: CB-10518
> URL: https://issues.apache.org/jira/browse/CB-10518
> Project: Apache Cordova
>  Issue Type: Bug
>  Components: CordovaLib
>Affects Versions: 6.0.0
>Reporter: Vladimir Kotikov
>Priority: Minor
>  Labels: triaged
>
> In case of fatal errors {{cordova run}} and {{cordova emulate}} methods emit 
> error messages using {{log}} event. This code probably should be changed to 
> use 'warn' level. (We can't use 'error' here due to its special meaning in 
> Node: https://nodejs.org/api/events.html#events_error_events)
> There is also other places, where 'log' events is used incorrectly:
> cordova-lib\src\cordova\targets.js:events.emit('log', 'An unexpected 
> error has occured');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'error while 
> generating cordova.js');
> cordova-lib\src\plugman\browserify.js:events.emit('log', 'Error 
> running platform version script');



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org