pear2/Net_RouterOS

Can't use created object during one session

aTanCS opened this issue · 10 comments

Hi, this code should create a logging action and create a rule with it. but the rule creation fails. Only after a reconnection (running script the second time) it creates the rule.

#this part runs only if syslog doesn't exist
$util->setMenu('/system logging action');
$add['name'] = 'syslog';
$add['remote'] = $syslog_ip;
$add['syslog-facility'] = $syslog_facility;
$add['bsd-syslog'] = 'yes';
$add['target'] = 'remote';
$res = $util->add($add);
#this returns 'syslog'
$name = $util->get($res, 'name');
#this part runs always
$util->setMenu('/system logging');
$add['action'] = 'syslog';
$add['topics'] = 'critical,error,warning,info';
#this returns empty script
$res = $util->add($add);

Looks like a RouterOS bug, but I shall double check anyway (later though...).

Could you try sleeping for 2 seconds before using the new action? e.g.

$res = $util->add($add);
sleep(2);
#this part runs always
$util->setMenu('/system logging');
$add['action'] = 'syslog';
$add['topics'] = 'critical,error,warning,info';
#this returns empty script
$res = $util->add($add);

I'm thinking that "/system logging add" is reading the actions from HDD, and not from memory. If even that doesn't work, chances are that RouterOS is doing the opposite - caching the logging actions in memory when the connection starts, and doesn't invalidate the cache when you add the new action.

As a workaround, maybe you could also try using the ID instead of the name? e.g.

$res = $util->add($add);
#this part runs always
$util->setMenu('/system logging');
$add['action'] = $res;
$add['topics'] = 'critical,error,warning,info';
#this returns empty script
$res = $util->add($add);

(I'll be surprised if this works, but it's worth the shot)

Delay didn't help.

Using ID also didn't work.

In case of ROS caching, would it be worth adding something like $util->reconnect() to force ROS to flush everything?

Interesting idea, but I'm not sure it's worth it, because it would require that the IP, port, username, password, encryption setting and context are all stored inside the Client object (they currently aren't), which means needless additional memory cost (a small one, admittedly, but still) per Client for the 99% of scenarios (where you don't need to reconnect). Also, it's certain to mess up persistent connections.

You could easily abstract away the connection into a function/method, and call that function/method when you need to reconnect. The fact that the old object is lost should cause the first connection to be closed shortly after the second one opens.

e.g.

function createRouterConnection() {
    return new RouterOS\Util(new RouterOS\Client('hostname', 'username', 'password'));
}

$util = createRouterConnection();

#this part runs only if syslog doesn't exist
$util->setMenu('/system logging action');
$add['name'] = 'syslog';
$add['remote'] = $syslog_ip;
$add['syslog-facility'] = $syslog_facility;
$add['bsd-syslog'] = 'yes';
$add['target'] = 'remote';
$res = $util->add($add);
$util = createRouterConnection();

#this part runs always
$util->setMenu('/system logging');
$add['action'] = 'syslog';
$add['topics'] = 'critical,error,warning,info';
$res = $util->add($add);

OK, I tried this... and I can't duplicate it on RouterOS 6.33.3 x86 (running on a VirtualBox VM on a Windows 10 x64 host with PHP 7.0.0). What's your RouterOS version and architecture? If it's earlier version than that, there's a good possibility MikroTik had found that issue, and has already fixed it. If it's the same, it could be an architecture specific bug.

Just for reference, the code I used in full:

<?php
use PEAR2\Net\RouterOS;

require_once 'PEAR2_Net_RouterOS-1.0.0b5.phar';

$util = new RouterOS\Util(new RouterOS\Client('ros.example.com', 'apifull', 'apifull'));

$syslog_ip = '127.0.0.1';
$syslog_facility = 'syslog';

$add = array();
#this part runs only if syslog doesn't exist
$util->setMenu('/system logging action');
$add['name'] = 'syslog';
$add['remote'] = $syslog_ip;
$add['syslog-facility'] = $syslog_facility;
$add['bsd-syslog'] = 'yes';
$add['target'] = 'remote';
$res = $util->add($add);
#this returns 'syslog'
$name = $util->get($res, 'name');
var_dump($name);
var_dump($res);

$add = array();
#this part runs always
$util->setMenu('/system logging');
$add['action'] = 'syslog';
$add['topics'] = 'critical,error,warning,info';
#this returns empty script
$res = $util->add($add);

var_dump($res);

which produces, as expected

string(6) "syslog"
string(2) "*A"
string(2) "*A"

(with the ID ever increasing at each repeated attempt, and the items also being visible from Winbox; The IDs are the same only because the VM doesn't have previously defined custom logging stuff; Adding and removing an item from just one menu before retrying the above confirms the two IDs are different)

OMG!!! Shame on me! Such a stupid mistake. Sorry. :( I forgot about resetting $add array before next request and it didn't rise any error. Is there any possibility to get API error if there are bad parameters in a request?

The problem is API errors can sometimes be several, and not just one and only...

I guess maybe I could make a new Exception class that would include the whole ResponseCollection, out of which you could then read the exact errors.

Hmmm... That may actually be quite nice... It would allow most other Util methods to be turned into fluent ones. Imagine

try {
    $util->setMenu('/....')->set(0, array('something'))->disable(1)->enable(2)->move(3,4)->setMenu('/....')->remove(5)->exec(':log info "YES!"')->add(array('stuff'));
} catch (RouterOS\UtilException $e) {/*tentative name...*/
    //see the first error message
    echo $e->getResponseCollection()/*tentative name...*/
        ->current()->getProperty('message');
}

This would be very helpfull!

Well, in the end, I didn't go as far as making the CRUD methods fluent, because I think if in the future, MikroTik adds useful info into successful output, there should be a way for it to be read.

But now, in the latest version, on failure, a RouterErrorException is thrown, making errors that much more "in your face".