It works sometimes, but often fails producing the log entries below. One such error indicates a “timeout.”
And, I assume that because of the timeout failure, the file “$identitydate-IP.txt” doesn’t exist and the command to remove it throws an error.
So, my question is whether it is best to handle the timeout situation or ignore the timeout situation and control for a situation where “$identitydate-IP.txt” doesn’t exist?
Or both.
And, of course, how to do this.
(Ignore “line 32” – I removed all sorts of commented out garbage for ease of readability – the file not found error refers to the 3rd to last line in the script.)
/system
:local cdate [clock get date]
:local yyyy [:pick $cdate 0 4]
:local MM [:pick $cdate 5 7]
:local dd [:pick $cdate 8 10]
:local identitydate "$[identity get name]"
# Set needed variables
:local username "<ME>"
:local clientkey "<CLIENTKEY>"
:local hostname "<MYDOMAIN>"
:global dyndnsForce
:global previousIP
# get the current IP address from the internet (in case of double-nat)
/tool fetch mode=http address="checkip.dyndns.org" src-path="/" dst-path="/dyndns.checkip.html"
:delay 1
:local result [/file get dyndns.checkip.html contents]
# parse the current IP result
:local resultLen [:len $result]
:local startLoc [:find $result ": " -1]
:set startLoc ($startLoc + 2)
:local endLoc [:find $result "</body>" -1]
:local currentIP [:pick $result $startLoc $endLoc]
:log info "UpdateDynDNS: currentIP = $currentIP"
#:set dyndnsForce true
# Determine if dyndns update is needed
# more dyndns updater request details https://help.dyn.com/remote-access-api/perform-update/
:log info "UpdateDynDNS: previousIP = $previousIP"
:if ($dyndnsForce = true) do={ :log warning "UpdateDynDNS: Forced update on" }
:if (($currentIP != $previousIP) || ($dyndnsForce = true)) do={
:set dyndnsForce false
:set previousIP $currentIP
/tool fetch mode=https \
url="https://$username:$clientkey@members.dyndns.org/v3/update?hostname=$hostname&myip=$currentIP" \
dst-path="/dyndns.txt"
:delay 3
:local result [/file get dyndns.txt contents]
:log info ("UpdateDynDNS: Dyndns update needed")
:log info ("UpdateDynDNS: Dyndns Update Result: ".$result)
:put ("Dyndns Update Result: ".$result)
/ip/address print file="$identitydate-IP"
/tool fetch upload=yes mode=ftp ascii=no src-path="$[$identitydate]-IP.txt" dst-path="/mikrotik-backups/$[$identitydate]-IP.txt" address=192.168.2.22 port=21 user=mikrotik password=mikrotik
/file remove "$identitydate-IP.txt"
} else={:log info ("UpdateDynDNS: No dyndns update needed")
}
Try with some delay after this command. Also correct filename, it is not the same that is being used for upload with fetch command after (not sure that .txt extension is added by default, check it) and remove square brackets for variable value in filename string or just use rewritten script by @rextended and no worries
I really want to understand this code, but these lines are just a little beyond my comprehension at this time.
Best I can figure out, “result” is assigned the value of the item/field/component of the fetch commant to checkup.dyndns.org/dyndne.checkip.html named “data”
But, how do you know that there is such array item?
I’ll stop talking now, because I suspect I’m making less and less sense…
The data element in ([/tool fetch ... as-value output=user]->"data") is defined by MikroTik as part of the return of /tool/fetch, so it should be there (assuming the as-value and output=user are both present).
And ->"data" contain the HTML returned by your DDNS service. So the 2nd line looks for text between : and </body> via the pick to get the data.
If you want your script to be reliable and handle errors (as topic title states), all code parts that can potentially fail should obviously be placed inside error handler
If your code was executed successfully, $execFail value will be false, meaning there is no error occured. If your code fails, this value will be true, indicating there is an error, and $err variable will contain a string with error message, that can be used for adding error message to the log inside error handling block. $execFail value can be used to decide, what code should be executed next, depending on result of executing the code inside error handler.
For details refer to :onerror command description in the docs:
There are many potential failure points in your code . For example, fetch command can fail simply because of connectivity issues, and in such case your script will fail entirely at this point. This is exactly what wee see on your screenshot of the log. It’s not related to file removal command as you assuming, it clearly states about timeout error while executing fetch command. So, this command should definitely be placed inside error handler.
But all stuff with file adding/reading/removal should also be placed inside error handlers, so the script won’t fail, if the file doesn’t exist or whatever.
Added some error handling, based on @rextended version.
:global dyndnsForce
:global previousIP
:local username "<ME>"
:local clientkey "<CLIENTKEY>"
:local hostname "<MYDOMAIN>"
:local idName [/system identity get name]
:local result
:local execFail
:local currentIP
:local ftpServer "192.168.2.22"; # just for convenience :) you won't need to search it in the code, if changed for some reason
# Use prefixes for logging, it's much more convenient, especially if you decide to edit it some day
:local ddnsPrefix "UpdateDynDNS:"
# You don't need to download data into file and then read this file. You can download the data directly into variable.
# url parameter already contains mode (http), address, src-path, no need to specify everything individually
:set execFail [
:onerror errMsg {
:set result [/tool fetch url="http://checkip.dyndns.org/dyndns.checkip.html" output=user as-value]
} do={
:log error "$ddnsPrefix Error occured while getting current IP address: $errMsg"
}
]
# If no error occured then continue
:if (!$execFail) do={
# Check fetch command execution status. Should be "finished", if everything is OK
:if ($result->"status" = "finished") do={
:set result ($result->"data")
:set currentIP [:pick $result ([:find $result ": " -1] + 2) [:find $result "</body>" -1]]
# Don't get the reason to use != false instead. This will lead to force update, if it's not defined...
:if ($dyndnsForce = true) do={
:log warning "$ddnsPrefix Forced update on"
}
:if (($currentIP != $previousIP) or ($dyndnsForce = true)) do={
:log info "$ddnsPrefix previousIP = $previousIP ; currentIP = $currentIP"
:set dyndnsForce false
:set previousIP $currentIP
# Why do you need to remove the file? It will be just overwritten by fetch command
# /file remove [find where name="dyndns.txt"]
:set execFail [
:onerror errMsg {
/tool fetch url="https://$username:$clientkey@members.dyndns.org/v3/update\3Fhostname=$hostname&myip=$currentIP" dst-path="dyndns.txt"
} do={
:log error "$ddnsPrefix Error getting DynDNS data: $errMsg"
}
]
:if (!$execFail) do={
:set execFail [
:onerror errMsg {
:set result [/file get "dyndns.txt" contents]
} do={
:log error "$ddnsPrefix Error reading DynDNS contents: $errMsg"
}
]
:if (!$execFail) do={
:onerror errMsg {
# Again, mode and path can be specified in url
/tool fetch url="ftp://$ftpServer/mikrotik-backups/$idName\2DIP.txt" upload=yes ascii=no user=mikrotik password=mikrotik \
src-path="dyndns.txt"
} do={
:log error "$ddnsPrefix Error uploading DynDNS data to FTP: $errMsg"
}
# Again, no need to remove the file, that will be overwritten each time
# /file remove [find where name="dyndns.txt"]
:log info "$ddnsPrefix Dyndns Update Result: $result"
}
}
} else={
:log info "$ddnsPrefix No dyndns update needed"
}
} else={
:log error ("$ddnsPrefix Error fetch status: ".($result->"status"))
}
}
And I would recommend to create a small ramdisk and download your dyndns.txt file there to avoid unneeded flash writing with temporary file.
Why not just break script execution with error using :error command with some message, like for error log, instead lots $execFail conditions with nested code?
As I see due to conditions checks no other executions are performed if $execFail is true so there is no actual need for such nesting.
Yes, can be done this way.
I just used to write scripts in infinite loop, if they are performing some repeated tasks, so I’m using my approach usually.
And if it’s not defined, it’s probably because the device has rebooted…
That’s exactly what I (is) wanted, if you restart the device it syncs when you turn it back on…
And at the same time, is useless ($dyndnsForce = true), just $dyndnsForce suffice…
Understood. Well, then it’s up to OP to decide what behavior he needs.
I don’t know, how $dyndnsForce is set in OP’s case, but if manually, it’s a potential place for typos. So, if this variable is non-bool, then :if ($dyndnsForce) will lead to error, and a whole script will fail. What’s interesting, if it’s undefined, there is no error, but if it’s a string, it fails.
[admin@xxx] > :global test
[admin@xxx] > :if ($test) do={:put "123"}
[admin@xxx] >
[admin@xxx] > :global test "truee"
[admin@xxx] > :if ($test) do={:put "123"}
conditional is not boolean
[admin@xxx] > :if ($test = true) do={:put "123"}
[admin@xxx] >
