karbassi / netatmo

A node.js module to hook into the netatmo API.
MIT License
57 stars 46 forks source link

Different type of error handling #36

Closed skrollme closed 4 years ago

skrollme commented 7 years ago

Hello, this is more a questions or a suggestion than a bug but I'm interested why the error-handling - for example in getstationsdata - is how it is.

This is the current code which ends up inside the first if-statement if an error occurs without calling the callback which includes an never-filled err-parameter:

request({
    url: url,
    method: "GET",
    form: form,
  }, function (err, response, body) {
    if (err || response.statusCode != 200) {
      return this.handleRequestError(err, response, body, "getStationsDataError error");
    }

    body = JSON.parse(body);

    var devices = body.body.devices;

    this.emit('get-stationsdata', err, devices);

    if (callback) {
      return callback(err, devices);
    }

Wouldn't it make more sense if the callback would be called everytime if given, with or without an err-object. In this case implementations could rely on callbacks. In my case I have a homebridge-plugin which polls the netatmo API regularly. Every now and then the Netatmo-API fails, I run into a soft-ban (too many calls) or get banned because I keep polling although the API return Error-500 because of some problems on the API server. I'm not a nodeJS-pro but I currently do not see a way how to handle the errors and still continue the normal logic.

So in my case something like this would be way more useful. What do you think?

}, function (err, response, body) {
    if (err || response.statusCode != 200) {
      this.handleRequestError(err, response, body, "getStationsDataError error");
      body = '{"body":{"devices":[]}}';
      err = (err)?err:true;
    }