wing icon indicating copy to clipboard operation
wing copied to clipboard

missing UI connection from Api to Bucket

Open ekeren opened this issue 1 year ago • 4 comments

I tried this:

bring cloud;

let api = new cloud.Api();
let b = new cloud.Bucket();
api.post("/doc/:id", inflight (req) => {
  b.put(req.vars.get("id"), req.body! );
});

This happened:

image

I expected this:

To see connection between api gateway to bucket

Is there a workaround?

No response

Anything else?

No response

Wing Version

0.73.41

Node.js Version

No response

Platform(s)

No response

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.

ekeren avatar May 12 '24 10:05 ekeren

We're missing information from the simulator. These are the connections listed:

[
	{
		"source": "root/Default/Api",
		"target": "root/Default/Api/OnRequestHandler0",
		"name": "post()"
	},
	{
		"source": "root/Default/$Closure1_0",
		"sourceOp": "handle",
		"target": "root/Default/Bucket",
		"targetOp": "put",
		"name": "put"
	}
]

So basically we're missing some connection from root/Default/Api/OnRequestHandler0 to root/Default/$Closure1_0.

@Chriscbr any insights?

skyrpex avatar May 13 '24 08:05 skyrpex

I did a bit of digging. In theory there should be another connection getting generated, but there's a little architectural issue:

  1. Connection data is currently associated with the app (construct tree). I think this is probably a good thing(?) - if you're unit testing apps and you create multiple apps aka construct trees, their connection data is separate as you'd expect. We also have other data associated with the construct tree, like platform parameters.
  2. Inflights created with the inflight() function here were changed in https://github.com/winglang/wing/pull/4993 to no longer be constructs, so they don't automatically have any access to the app or construct tree. This means there's no way for an inflight to add info to the tree about its connection data. This limitation was only really uncovered when we started using the inflight() function within the SDK's source code in https://github.com/winglang/wing/pull/6354.

I feel like the best solution here would be to make inflights constructs again. It would also let us clean up this issue where token resolvers are globals, which seems like it might cause issues in the future. But I'm curious what y'all think @MarkMcCulloh @eladb

Chriscbr avatar May 13 '24 22:05 Chriscbr

there's no way for an inflight to add info to the tree about its connection data

Could the inflight host (e.g. OnRequestHandler) add the connection itself? It knows what connections the inflight has (lifts) so maybe it can link itself to them. Inflights themselves are just some data that happens to relate itself to the construct tree. It would be like representing arbitrary strings as constructs just because could have a possible relationship to the tree (via tokens). I wouldn't die on this hill but philosophically, non-construct inflights make more sense to me.

MarkMcCulloh avatar May 13 '24 22:05 MarkMcCulloh

note: It looks the issue of broken extension also applies whenever you create an inflight using the TypeScript framework:

import { cloud, inflight, lift, main } from "@wingcloud/framework";
import assert from "node:assert";

main((root, test) => {
  const bucket = new cloud.Bucket(root, "Bucket");
  const fn = new cloud.Function(
    root,
    "Function",
    lift({ bucket }).inflight(async (ctx) => {
      ctx.bucket.put("key", "value");
      return "hello, world";
    })
  );

  test(
    "fn returns hello",
    lift({ fn }).inflight(async ({ fn }) => {
      assert.equal(await fn.invoke(""), "hello, world");
    })
  );
});
Screenshot 2024-05-14 at 1 14 03 PM

With the same code in Wing it works:

bring cloud;

let bucket = new cloud.Bucket();
let fn = new cloud.Function(inflight () => {
  bucket.put("key", "value");
  return "hello, world";
});

test "fn returns hello" {
  assert(fn.invoke() == "hello, world");
}
Screenshot 2024-05-14 at 1 16 33 PM

Could the inflight host (e.g. OnRequestHandler) add the connection itself? It knows what connections the inflight has (lifts) so maybe it can link itself to them.

I'd like to take you up on this suggestion. Putting it another way, we'd be making it the responsibility of the inflight host to register any connection data while it's lifting the inflight. It feels like a logical place to do the work. It would mean that a free-floating inflight closure (that doesn't get used anywhere) wouldn't emit any connection data, which also feels right. I imagine the logic can be included as part of the lifting stuff here: https://github.com/winglang/wing/blob/18cce7375bd736474276e2945d203f14e647e15c/libs/wingsdk/src/cloud/function.ts#L123-L125

Chriscbr avatar May 14 '24 18:05 Chriscbr

Just making sure we are prioritizing this issue, currently even our classic getting started example looks "broken"

image

ekeren avatar May 16 '24 12:05 ekeren

Congrats! :rocket: This was released in Wing 0.73.52.

monadabot avatar May 17 '24 04:05 monadabot