This repository has been archived by the owner. It is now read-only.

Fix exposing the error #15

Merged
merged 1 commit into from Sep 2, 2015

Conversation

Projects
None yet
2 participants
@stof
Contributor

stof commented Sep 2, 2015

Using a promise for the setTimeout call is broken in this case, as the error would again be thrown in a promise operation, and so not exposed

Fix exposing the error
Using a promise for the setTimeout call is broken in this case, as the error would again be thrown in a promise operation, and so not exposed
@javallone

This comment has been minimized.

Show comment
Hide comment
@javallone

javallone Sep 2, 2015

Owner

Do you have an example of this not working? I added some throw statements into portions of code that use exposeError and the error was correctly reported in the browser console.

Owner

javallone commented Sep 2, 2015

Do you have an example of this not working? I added some throw statements into portions of code that use exposeError and the error was correctly reported in the browser console.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Sep 2, 2015

Contributor

I guess you added it in places which are not running inside a promise but before the promise is used (and util.exposeError was not involved at all in the error handling then). And so the error does not cause the method to return a rejected promise on error but to trigger an exception instead. This is an issue which happens when the API is partially synchronous and partially promised-based.

Try this patch to trigger an error in a place running inside a promise:

diff --git a/src/js/regexper.js b/src/js/regexper.js
index fea7c4b..27b2a9b 100644
--- a/src/js/regexper.js
+++ b/src/js/regexper.js
@@ -226,6 +226,7 @@ export default class Regexper {
       })
       // When parsing is successful, render the parsed expression.
       .then(parser => {
+        throw new Error('Debug');
         return parser.render();
       })
       // Once rendering is complete:

The error will not appear in the console because the exception triggered in exposeError just cause the tick().then(...) promise to get rejected but not cause an asynchronous error.

Contributor

stof commented Sep 2, 2015

I guess you added it in places which are not running inside a promise but before the promise is used (and util.exposeError was not involved at all in the error handling then). And so the error does not cause the method to return a rejected promise on error but to trigger an exception instead. This is an issue which happens when the API is partially synchronous and partially promised-based.

Try this patch to trigger an error in a place running inside a promise:

diff --git a/src/js/regexper.js b/src/js/regexper.js
index fea7c4b..27b2a9b 100644
--- a/src/js/regexper.js
+++ b/src/js/regexper.js
@@ -226,6 +226,7 @@ export default class Regexper {
       })
       // When parsing is successful, render the parsed expression.
       .then(parser => {
+        throw new Error('Debug');
         return parser.render();
       })
       // Once rendering is complete:

The error will not appear in the console because the exception triggered in exposeError just cause the tick().then(...) promise to get rejected but not cause an asynchronous error.

@javallone

This comment has been minimized.

Show comment
Hide comment
@javallone

javallone Sep 2, 2015

Owner

Thank you for the test case.

Owner

javallone commented Sep 2, 2015

Thank you for the test case.

javallone added a commit that referenced this pull request Sep 2, 2015

Merge pull request #15 from stof/patch-1
Fix exposing the error

@javallone javallone merged commit d3ffd3a into javallone:master Sep 2, 2015

@stof stof deleted the stof:patch-1 branch Sep 3, 2015

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