[BUG] Returning SlackAPIClient from an async function crashes Deno

[manterfield]

The deno-slack versions

I don't have an import map, but deno_slack_api is 2.7.0 - also tested at 2.1.1

Deno runtime version

deno 1.44.4+86010be

OS info

ProductName:		macOS
ProductVersion:		14.4.1
BuildVersion:		23E224
Darwin Kernel Version 23.4.0: Fri Mar 15 00:10:42 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6000

(also tried on Alpine-1.43.2)

Describe the bug

Attempting to return SlackAPIClient from an async function causes Deno to fail with a top-level await promise never resolved. I've also ran this within a worker (my original production code) and in that scenario execution seems to simply... stop. No errors thrown.

Steps to reproduce

// mod.ts

import { SlackAPI } from "https://deno.land/x/[email protected]/mod.ts";

const getClientAsync = async () => {
  // This return style always fails
  return SlackAPI("foo");
};

const getClientNestedAsync = async () => {
  // Nesting it in an object succeeds
  return { c: SlackAPI("foo") };
};

const getClient = () => {
  // Returning from a non-async function succeeds
  return SlackAPI("foo");
};

const run = async () => {
  console.log("Running");
  SlackAPI("foo");
  console.log("Got inline client");

  getClient();
  console.log("Got client from func");

  await getClientNestedAsync();
  console.log("Got client from nested async func");

  await getClientAsync();
  console.log("Got client from async func");
};

if (import.meta.main) {
  await run();
}

Run with: deno run --allow-all mod.ts

Expected result

Should see all logs, including Got client from async func. More generally, I should be able to return the SlackClientAPI object without error from an async function.

Actual result

Running the above outputs:

Running
Got inline client
Got client from func
Got client from nested async func
error: Top-level await promise never resolved
  await run();
  ^
    at <anonymous> (file:///mod.ts:34:3)

If I move the call to getClientAsync anywhere else, it still immediately fails at the point it makes that call.

Note:

I've also raised an issue on Deno here: denoland/deno_core#828

It seems to me that Deno shouldn't behave this way, but also it might be something this project can workaround (since the SlackAPIClient is the first value I've seen exhibit this behaviour).

[manterfield]

It looks to be due to how the nested proxy logic works here, returning a function for any prop via the getter:

deno-slack-api/src/api-proxy.ts

Lines 46 to 57 in 26fcdd6

	const proxy = new Proxy(objectToProxy, {
	get(obj, prop) {
	// We're attempting to access a property that doesn't exist, so create a new nested proxy
	if (typeof prop === "string" && !(prop in obj)) {
	return APIProxy(null, apiCallback, ...path, prop);
	}
	// Fallback to trying to access it directly
	// deno-lint-ignore no-explicit-any
	return Reflect.get.apply(obj, arguments as any);
	},
	});

Deno calls then on the returned object to check if it's a nested promise. At that point it blows up as accessing then returns the apiCallback function - meaning Deno interprets this as a promise.

[manterfield]

As a workaround for anyone else having this issue:

const c = SlackAPI("foo");
Object.assign(c, { then: undefined });
return c;

This stops it from blowing up

[filmaj]

Hey @manterfield, thanks for filing this.

Deno calls then on the returned object to check if it's a nested promise.

Can you link me to code showing this?

Maybe the proxy getter in this module is a bit too sweeping / far-reaching? Perhaps there should be a list of methods/properties it doesn't try to direct to apiCallback - e.g., Promise instance methods.

My two questions above are trying to tease out an answer to the question "where should a fix for this problem land?"

[manterfield]

@filmaj

I agree that the proxy getter is a bit too greedy in this case.

So if you add a console.log(obj, prop) to line 48 here:

deno-slack-api/src/api-proxy.ts

Line 48 in 26fcdd6

// We're attempting to access a property that doesn't exist, so create a new nested proxy

You'll see it output then for the prop being accessed (as long as the client is returned from an async function as in my example above).

I researched after bumping into this and it isn't Deno specific. The spec actually describes this behaviour: https://promisesaplus.com/

“promise” is an object or function with a then method whose behavior conforms to this specification.
“thenable” is an object or function that defines a then method.

and the promise resolution behaviour:

The promise resolution procedure is an abstract operation taking as input a promise and a value, which we denote as [[Resolve]](promise, x). If x is a thenable, it attempts to make promise adopt the state of x, under the assumption that x behaves at least somewhat like a promise. Otherwise, it fulfills promise with the value x.

Which makes perfect sense now I've been forced to think about it, though I had been quite enjoying my ignorance. If I directly return the result of an async function from an async function, I wouldn't want to have to resolve twice.

[filmaj]

Thanks for the discussion <3 I agree. I'll work on a patch to reduce the scope of proxy greed :D

[filmaj]

Fixed in deno_slack_api 2.7.1: https://github.com/slackapi/deno-slack-api/releases/tag/2.7.1
deno_slack_api 2.7.1 is also included in the freshly-released deno_slack_sdk 2.14.1: https://github.com/slackapi/deno-slack-sdk/releases/tag/2.14.1