denoland / deno_core

The core engine at the heart of Deno
MIT License
252 stars 83 forks source link

Returning certain values asynchronously causes Deno to fail to resolve promise #828

Open manterfield opened 1 month ago

manterfield commented 1 month ago

Disclaimer: I could totally be doing something stupid here, but I can't see what.

Code that reproduces the issue for me:

// mod.ts

import { SlackAPI } from "https://deno.land/x/deno_slack_api@2.1.1/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 this with: deno run --allow-all mod.ts Deno version: deno 1.44.4+86010be

If I run the above code, I get this output:

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:///Users/tm/Code/scratch/denobug/mod.ts:31:3)

I can swap the order of the getClient calls around - it always fails at the call to the async, non-nested return.

I first saw this behaviour in a worker, where it doesn't even show the error. It just seems to... stop. No error, nothing to show it has crashed - I have a lot going on in my worker though so can't rule out suppression.

I've also tested the returned client in all cases, and in all cases the client is functional/can make calls as expected. I've tried every odd variation I can think of, including assigning the client to a variable, using it first, then returning.

It seems that no matter what, if that specific value is returned from an async function, it crashes Deno and can not be caught.

marvinhagemeister commented 1 month ago

Smaller reproduction case:

import { SlackAPI } from "https://deno.land/x/deno_slack_api@2.1.1/mod.ts";
await SlackAPI("foo");

Error:

error: Top-level await promise never resolved
await SlackAPI("foo");
^
    at <anonymous> (file:///Users/marvinh/dev/test/deno-slack/main.ts:3:1)
manterfield commented 1 month ago

@marvinhagemeister Having dug into the Slack code more and taught myself a bit more about Proxies and nested promises, I'm not sure this is an issue with Deno.

The Slack code returns a Proxy with a getter. Deno checks if this is thenable, which is caught by the getter which in turn will eventually return a proxied function (so it appears as if it's a promise).

I suppose Deno could potentially help with making it clear what just happened, but I wonder if this is expected behaviour other than that.

manterfield commented 1 month ago

Slack code that's relevant is here: https://github.com/slackapi/deno-slack-api/blob/26fcdd6645c96998a0d3870d325edf476ae83716/src/api-proxy.ts#L46-L57

Just so you can judge if this is Deno acting properly in the face of that.