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

Throw error for invalid exception handler #5271

Closed
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
2 participants
@aramk
Contributor

aramk commented Sep 24, 2015

Meteor.bindEnvironment() can have an invalid error handler accidentally passed as the second argument, especially in CoffeeScript where the issue is not easy to find. This can cause a cryptic error message about an exception being thrown when onException() itself is being called.

Throw error for invalid exception handler
Meteor.bindEnvironment() can have an invalid error handler accidentally passed as the second argument, especially in CoffeeScript where the issue is not easy to find. This can cause a cryptic error message about an exception being thrown when onException() itself is being called.
@avital

This comment has been minimized.

Contributor

avital commented Oct 6, 2015

Sure, this looks good

@avital

This comment has been minimized.

Contributor

avital commented Oct 6, 2015

I'm curious -- what about CoffeeScript makes it easy to accidentally make this mistake?

avital added a commit that referenced this pull request Oct 6, 2015

@avital

This comment has been minimized.

Contributor

avital commented Oct 6, 2015

Closed via 0e22996

@avital avital closed this Oct 6, 2015

@aramk

This comment has been minimized.

Contributor

aramk commented Oct 7, 2015

Thanks for that! I had done this:

updateProjectModifiedDate = _.throttle Meteor.bindEnvironment (projectId, userId) ->
  Projects.update projectId, $set: {dateModified: getCurrentDate(), userModified: userId}
, 5000

I expected the 5000ms to be applied to _.throttle() but the output is:

updateProjectModifiedDate = _.throttle(Meteor.bindEnvironment(function(projectId, userId) {
  return Projects.update(projectId, {
    $set: {
      dateModified: getCurrentDate(),
      userModified: userId
    }
  });
}, 5000));

Since it passes 5000 to Meteor.bindEnvironment() you get a strange error saying a number is not a function if an error is thrown from within. The corrected coffeescript was:

updateProjectModifiedDate = Meteor.bindEnvironment (projectId, userId) ->
  Projects.update projectId, $set: {dateModified: getCurrentDate(), userModified: userId}
updateProjectModifiedDate = _.throttle(updateProjectModifiedDate, 5000)
@avital

This comment has been minimized.

Contributor

avital commented Oct 7, 2015

Oh coffeescript...

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