Spark.function() args > 63 chars knocks Core offline briefly

[technobly]

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.

[m-mcgowan]

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!

[zsup]

I think any response is better than dropping the cloud connection. Although between your first three options I don't have a strong preference

[dmiddlecamp]

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?

[andyw-lala]

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 <notifications@github.com

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
#30 (comment)
.

Andy

[andyw-lala]

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 andyw@pobox.com 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 <
notifications@github.com> 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
#30 (comment)
.

Andy

Andy

[m-mcgowan]

The Spark.log() proposal addresses communicating errors to the cloud, so that would be put to good use here.

[andyw-lala]

+1

On Tue, Apr 14, 2015 at 10:37 AM, Matthew McGowan notifications@github.com
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
#30 (comment)
.

Andy

[andyw-lala]

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 andyw@pobox.com wrote:

+1

On Tue, Apr 14, 2015 at 10:37 AM, Matthew McGowan <
notifications@github.com> 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
#30 (comment)
.

Andy

Andy

[m-mcgowan]

Yep to all of that. afaik that's the direction we're heading. api errors will be distinct from other logs.

[andyw-lala]

+10^6

On Tue, Apr 14, 2015 at 10:55 AM, Matthew McGowan notifications@github.com
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
#30 (comment)
.

Andy