Spark.function() args > 63 chars knocks Core offline briefly
Tried with 0.3.4 and the latest and greatest HAL firmware and the Core is doing the same thing, dropping off the cloud for a good 1 second count if I throw more than 63 chars at the API for Spark.function()
@dmiddlecamp has confirmed no hard limit for this one in the API, so it sounds like the limit must be causing this response at the firmware level.
It seems like a hard coded limit in the API would be good:
Pros:
- saves BW
- can return a lovely error message "stahp sending me more than 63 chars!"
Cons:
- if different devices have different limits, more CPU usage on server
- ?
I also think the firmware should have a hard coded limit that keeps it operating sanely.
The cause of the drop-off the cloud is this check here:
https://github.com/spark/core-communication-lib/blob/master/src/spark_protocol.cpp#L887-L890
If the function argument passed is longer than the limit - currently 63 characters then return false.
The function returns false on error (typically read/write error), which causes the caller to drop the device off the cloud. (Sets SPARK_CLOUD_SOCKETED=0, so the next iteration of the background loop, the cloud is reconnected.)
So the question is, rather than dropping the cloud connection, is there a better response?
- return true, and not send a response. This will cause the cloud to eventually timeout.
- return true, and send a result of -1.
- truncate the parameter?
- keep as is - it's a caller problem, they can sanitize their data!
I think any response is better than dropping the cloud connection. Although between your first three options I don't have a strong preference
I think ideally the device should return 'FunctionReturnError' to the cloud code: Message.Code.BAD_REQUEST, type: Message.Type.NON, and not drop the connection?
Since the only UI on the core is a blinky LED, and that might be in a box hundreds of miles away, how about we generate a generic exception reporting mechanism, that the core can use to say "I was asked to do X, which is not good" - this would take some time to actually design well, but combined with the dashboard, things could get very useful very quickly.
Bonus points for immediately including a cloud-reported ASSERT() and/or debug_printf() mechanism.
On Tue, Apr 14, 2015 at 10:05 AM, David Middlecamp <[email protected]
wrote:
I think ideally the device should return 'FunctionReturnError' to the cloud code: Message.Code.BAD_REQUEST, type: Message.Type.NON, and not drop the connection?
— Reply to this email directly or view it on GitHub https://github.com/spark/core-communication-lib/issues/30#issuecomment-92894212 .
Andy
Oh - and return some safe, non-pathological return value, and do not kick the core offline.
On Tue, Apr 14, 2015 at 10:10 AM, Andy Warner [email protected] wrote:
Since the only UI on the core is a blinky LED, and that might be in a box hundreds of miles away, how about we generate a generic exception reporting mechanism, that the core can use to say "I was asked to do X, which is not good" - this would take some time to actually design well, but combined with the dashboard, things could get very useful very quickly.
Bonus points for immediately including a cloud-reported ASSERT() and/or debug_printf() mechanism.
On Tue, Apr 14, 2015 at 10:05 AM, David Middlecamp < [email protected]> wrote:
I think ideally the device should return 'FunctionReturnError' to the cloud code: Message.Code.BAD_REQUEST, type: Message.Type.NON, and not drop the connection?
— Reply to this email directly or view it on GitHub https://github.com/spark/core-communication-lib/issues/30#issuecomment-92894212 .
Andy
Andy
The Spark.log() proposal addresses communicating errors to the cloud, so that would be put to good use here.
+1
On Tue, Apr 14, 2015 at 10:37 AM, Matthew McGowan [email protected] wrote:
The Spark.log() proposal addresses communicating errors to the cloud, so that would be put to good use here.
— Reply to this email directly or view it on GitHub https://github.com/spark/core-communication-lib/issues/30#issuecomment-92914960 .
Andy
Although we might want to partition categories of logs (think syslog, but -lite), so that API violations, user asserts, system asserts, etc were not all dumped together with the output from user log calls, and could be flagged as distinct in the dashboard & audit/log trails associated with the core/user.
Note that this also flips into fleet management - anything that suggests the app might be misbehaving should be able to be redirected to the fleet owner/admin, not the user.
On Tue, Apr 14, 2015 at 10:47 AM, Andy Warner [email protected] wrote:
+1
On Tue, Apr 14, 2015 at 10:37 AM, Matthew McGowan < [email protected]> wrote:
The Spark.log() proposal addresses communicating errors to the cloud, so that would be put to good use here.
— Reply to this email directly or view it on GitHub https://github.com/spark/core-communication-lib/issues/30#issuecomment-92914960 .
Andy
Andy
Yep to all of that. afaik that's the direction we're heading. api errors will be distinct from other logs.
+10^6
On Tue, Apr 14, 2015 at 10:55 AM, Matthew McGowan [email protected] wrote:
Yep to all of that. afaik that's the direction we're heading. api errors will be distinct from other logs.
— Reply to this email directly or view it on GitHub https://github.com/spark/core-communication-lib/issues/30#issuecomment-92927134 .
Andy