Exception handling best practices

[aadams]

Thanks. That means a lot.

[aadams]

Will someone please review this code for ‘correctness’?

@serkandurusoy, I was reading over some of your stack overflow and aldeed repo posts, will this work, since the client is async (it’s async even without a callback)?

client.js

'client #did-something': function (event, instance) {
  var some_value = Meteor.find('#some-value');
  try {
    Meteor.call('do_something', some_value, function (err, return_value) {
      if (err) throw new Meteor.Error('bubble up this error', err);
      else Session.set('got_it', return_value);
    });
    
    // do other stuff here
  } catch (e) {
    toastr.error(e);
    console.log('error bubbled up from do_something method call');
    console.log(e);
  }
}

lib/methods.js
// SIMULATED server method call

do_something: function (some_value) {
  // do stuff
  var documentId = ''
  
  // unblock so the client continues?
  this.unblock();

  try {
    documentId = good_collection.insert(some_value); 
    // do something here that takes a long time, then return to the client
  } catch (e) {
    throw new Meteor.Error(555, e);
  }
)

If an error gets thrown in the method, will the exception bubble up to the try/catch in the client?

And on the publish/subscribe side, does the subscribe callback get the error from the publish, is this the ‘right’ way?

client.js

Template.template_name.onCreated(function() {
  var instance = this;

  instance.autorun(function() {
      var subscription = instance.subscribe('get_collection', function(err) {
        if (err) {
         console.log('error from the publish,', err); 
       }
      });
      if (subscription.ready()) {
        console.log("received user cursor. \n\n")
      } else {
        console.log("subscription is not ready yet. \n\n");
      }
  });

  instance.the_collection = function() {
    var obj = my_collection.findOne({
      _id: Session.get('session_id')
    });

    return obj;
  }
});

should we use this.error here, instead of Meteor.Error?
server.js

Meteor.publish('get_collection', function() {
  var self = this;
  if (self.userId) {
    try {
      return my_collection.find({});
    } catch (e) {
      this.error(555, 'publish error');
    }
  }
  return self.ready();
});

[corvid]

This answer on stack overflow is helpful

Meteor.methods returns undefined

[serkandurusoy]

I think your use of try catch is too verbose and perhaps even overused.

Namely, if you are throwing Meteor.error you are already dealing with it on the callback and the outer try catch are then only useful if there is a vm based one-in-a-million wonky error.

http://docs.meteor.com/#/full/meteor_error

If you want to return an error from a method, throw an exception. Methods can throw any kind of exception. But Meteor.Error is the only kind of error that a server will send to the client. If a method function throws a different exception, then it will be mapped to a sanitized version on the wire. Specifically, if the sanitizedError field on the thrown error is set to a Meteor.Error, then that error will be sent to the client. Otherwise, if no sanitized version is available, the client gets Meteor.Error(500, ‘Internal server error’).

Basically, if meteor provides you with a callback function, it already provides you with the necessary error handling mechanism. And they always do that. Subscriptions, inserts, methods and all.

I think try/catch is more valuable around your custom functions that reside outside methods. And I think even they should throw Meteor.error if you want to somehow present them to the user.

Regarding try/catch wrapping around method calls on the client, it is just useless because a meteor method will always return undefined on the client.

I hope I understood your questions and been able to answer it. Otherwise, I’d be happy to continue the discussion.

[aadams]

Namely, if you are throwing Meteor.error you are already dealing with it on the callback and the outer try catch are then only useful if there is a vm based one-in-a-million wonky error.

alas, code would have been nice…

What I think you’re saying is, from the client’s perspective, and in terms of the relationship between the [Meteor.call] and [Meteor.method]:

---

Since I’m already (kinda have to) catching the [Meteor.Error], from the [Meteor.call], in the [Meteor.call]-callback, why not just deal with the error there as opposed to wrapping the [Meteor.call] in a try catch and dealing with it in the catch?

My answer is simply because in a lot of my Template Events, a lot of the time I have two or more [Meteor.calls], and I’d like to deal with all the exceptions in one spot. If I have to do a re-throw, that’s not a bad thing IMO.

[khamoud]

Here is one way that I handle exceptions with code.

// client side
Template.someTemplate.events({
  'click .someButton':function(){
    Meteor.call("someFunction", function(error, res){
      if(error){
        Session.set("errorState", true);
        Session.set("errorMessage", error.reason);
      }
    });
  }
})

// server side
Meteor.methods({
  someFunction:function(){
   if(true) throw new Meteor.Error("test-error", "This function will always throw an error");
   
   return "This line will never get executed";
 }
});

[aadams]

Right – that’s the way all my Meteor.call <-> Meteor.methods looked for a while.

Then I started not catching a few errors in my Meteor.methods because I didn’t have a try around everything.

So now I do both in the Meteor.method:

// simulation
Meteor.methods({
  someFunction: function () {
    if (error_condition_I_can_anticipate) 
      throw new Meteor.Error(505, 'I anticipated this!')

   // errors I might not anticipate wrap in try-catch
   try {
     // could do things that could cause an exception to be thrown
   } catch (e) {
     // this will bubble to the client and got get swallowed by the server
     Meteor.Error(500, 'unanticipated error', e);
   }
 }
});

What’s wrong with using a try-catch to catch unanticipated errors?

[serkandurusoy]

a lot of my Template Events, a lot of the time I have two or more [Meteor.calls], and I’d like to deal with all the exceptions in one spot. If I have to do a re-throw, that’s not a bad thing IMO.

You could try something like https://www.discovermeteor.com/blog/meteor-pattern-two-tiered-methods/ and instead of wrapping multiple method calls in a try/catch, you could try wrapping them in a method call that bundles other methods.

The benefits would be:

• Access to stubs (you’ll be able to receive back simulated return values since methods in methods, as opposed to standalone methods, do actually return simulated results instead of undefined
• Ability to bubble up all errors and the results up to a single point of entry where you can further handle the output either like the example @khamoud provided above, or perhaps with a dedicated error handling package https://atmospherejs.com/?q=error all native to meteor

[serkandurusoy]

Oh my! @aadams check this out https://github.com/meteor/meteor/blob/devel/History.md

On the client, Meteor.call now takes a throwStubExceptions option; if set, exceptions thrown by method stubs will be thrown instead of logged, and the method will not be invoked on the server. #4202

[aadams]

You could try something like https://www.discovermeteor.com/blog/meteor-pattern-two-tiered-methods/ and instead of wrapping multiple method calls in a try/catch, you could try wrapping them in a method call that bundles other methods

Thanks for the suggestion.

But would you mind writing a quick example? I’m a little confused by the article and don’t know how to square it with what you suggested.

[serkandurusoy]

Something like this should cover your case:

Meteor.methods({
  doThing: function(thing) {
    return Meteor.call("doOtherThing", thing, function(err,res) {
      if (res) return res;
    });
  },
  doOtherThing: function(firstThing){
    return firstThing + "and other thing";
  },
  doThings: function(thing){
    Meteor.call("doThing", thing, function(err,res) {
      if (res) return res;
    })
  }
})

Template.things.events({
  'click .thing': function(e,t) {
    Meteor.call("doThings", "thing", function(err,res) {
      if (err) {
        /* here, you have access to the first error that blocks this chain of method calls */
      }
    })
  }
})

I have not tested this code, but the logic is what I’ve been referring to, so it should work out just fine if there are no typos or js grammar mistakes there.

[techplex]

Have you all seen this solution? http://takidashi.com/smooth-error-handling-for-meteor-methods.html

// in /lib needed on both client and server

var throwError = function(error, reason, details) {
  error = new Meteor.Error(error, reason, details);
  if (Meteor.isClient) {
    return error;
  } else if (Meteor.isServer) {
    throw error;
  }
};

// Some Method

Meteor.methods({
  createPost: function(text) {
    if (!this.userId) {
      return throwError(403, 'Must be logged in');
    }
  }
});

Template.new_post.events({
  'click input#create_post': function(event, template) {
    var text;
    text = ($(event.target)).prev('input').val();
    return Meteor.call('createPost', text, function(error, result) {
      if (error) {
        return new Message(error.reason); //handle the message on the client
      }
    });
  }
});

[mitar]

So there is still no easy way to simply log all errors in methods in publish functions?

[crapthings]

use two collections, one for server and client, one for client only

[petr24]

Hey @mitar

I know this is an old post, but I was also looking for an answer on how to capture the Meteor.Error event. After a lot of googling around, I could not find any answers anywhere, so after working a little bit on the problem I think I might have something. Anyone else that sees this, maybe it helps you or you might have some feedback on the solution.

The idea for solution came from seeing a comment somewhere on github about how Kadira handled catching JS errors. Can’t find it right now, but will link to it when I do.

Here is what I did. I put this in the server dir this way it only catches errors thrown server side, but you could have this in lib dir.
I put the Logger in the function as an example.
Hope this helps.

var originalMeteorError = Meteor.Error;

//This lets us "catch" anytime a metoer error is thrown
Meteor.Error = function (error, reason, details) {

    Logger.log({
        level: "error",
        error: error,
        reason: reason,
        details: details
    });
    
    return originalMeteorError.apply(this, arguments);
};