SkygearIO / chat

Skygear Plugin - Chat SDK
Apache License 2.0
17 stars 18 forks source link

Record Save Bug Fix #188

Closed howawong closed 6 years ago

howawong commented 6 years ago

Only send record:save if len(args) > 0, otherwise server returns "expected list of record"

Connects SkygearIO/chat-SDK-Android#64

carmenlau commented 6 years ago
  1. Agreed we should handle saving empty list case in plugin, but also want to know if the calling of mark_as_delivered in android expected?

  2. When reviewing save of Database, makes me think of a problem. For the chat plugin, we usually use zmq or http for plugin? In the old version, http transmitter will raise error but now it will not. I think it will also affect the chat plugin here?

https://github.com/SkygearIO/py-skygear/commit/631d1f267a6b4cae1c0a4fc89dc3e72ee90ab19e

howawong commented 6 years ago

@carmenlau

  1. In reality and general, calling a function with an empty list cannot be prevented. So I think it is an expected behaviour.

  2. Then database is not the only place being affected. But it is a separate issue?

carmenlau commented 6 years ago

@howawong @rickmak

For the empty response, the function save return the result of send_action. The result of send_action is a dictionary instead of list. So it is not consistent to the empty result (empty list). Maybe we should not return the send_action result directly and remove the result key? If so, we also need to handle server error case, the result will be dictionary with error key.

rickmak commented 6 years ago

Sorry, I mean to keep the consistent output across 0 and 1+. So simply return [] is not match with the send_action response. Please make them align.

And I think for error case, we can raise an exception?