slackapi / deno-slack-sdk

SDK for building Run on Slack apps using Deno
https://api.slack.com/automation
MIT License
155 stars 27 forks source link

[BUG] Non-existing items are returning an empty item instead of error with client.apps.datastore.get #268

Closed blueprismo closed 4 months ago

blueprismo commented 7 months ago

The deno-slack versions

❯ deno --version
deno 1.39.0 (release, aarch64-apple-darwin)
v8 12.0.267.8
typescript 5.3.3
❯ slack --version
Using slack v2.16.0
---
 cat import_map.json| grep deno-
    "deno-slack-sdk/": "https://deno.land/x/deno_slack_sdk@2.5.0/",
    "deno-slack-api/": "https://deno.land/x/deno_slack_api@2.1.2/",

OS info

ProductName:            macOS
ProductVersion:         14.2.1
BuildVersion:           23C71
Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000

Describe the bug

Whenever I try to query the datastore with the get method, I got an empty item: {"ok":true,"datastore":"thxcoins","item":{}} Whereas in the documentation You can clearly see that a non-existing item should return the not_found error.

Steps to reproduce

const getResponse = await client.apps.datastore.get<typeof ThxDataStore.definition>({
      datastore: "thxcoins",
      id: "FooBar",
    });
    console.log(JSON.stringify(getResponse));
    if (!getResponse.ok){
        console.log("Error: " + JSON.stringify(getResponse.error))
      }
    } else {
      console.log("User found")
    }

Expected result

throw new Error("not_found");

Actual result

empty item (id does not exist!) {"ok":true,"datastore":"thxcoins","item":{}}

Requirements contributing is capitalized, broken link 😄 , but yes agree agree accept next next Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

filmaj commented 7 months ago

Thanks for the report, we are investigating whether this is a documentation issue or faulty behaviour. Will update once we have clarity.

blueprismo commented 6 months ago

Friendly ping :)

blueprismo commented 4 months ago

Friendly ping 2 :)

filmaj commented 4 months ago

@blueprismo I am so sorry for the late response, I totally overlooked getting back to you 😢

So: the docs are incorrect! Indeed the HTTP API returns an empty object {} with ok:true in the case that an item was not found. I will follow up to get those docs updated. We are also discussing the possibility of having the SDK abstract this discrepancy, but it would require a major new version of this SDK, as it would be a breaking change. Something to consider and let me know if you have opinions on that. We think that introducing a breaking change at the HTTP API level would be too disruptive for existing apps today.

blueprismo commented 4 months ago

@filmaj Thanks a lot for the assertiveness of the response ^^ From your POV the low-hanging fruit would be just to update the docs and state that will return and empty object. Even though my opinion is irrelevant, Maybe another soft change you could do is add a boolean property like empty to check if it returned a empty object, which would be a non-disruptive workaround.

filmaj commented 4 months ago

The docs have now been updated: https://api.slack.com/methods/apps.datastore.get

Going to close this, but if you have further issues here, feel free to re-open or file a new issue.