Support callFunction API #850

samouri · 2020-04-23T17:03:55Z

summary
The ability to invoke arbitrary worker functions from the main thread and get the result would be useful to two AMP use-cases, Protocol Adapters and Allow real JS in amp-bind.

This PR exposes two new interfaces. A function named callFunction invoking functions on the worker, as well as a method for the worker code to mark which functions should be exposed to the main thread. I implement this by changing the return value from upgradeElement from a standard Worker, into a wrapper object. Notably, it still delegates the onerror and .terminate() function calls to the underlying worker.

// index.html
<div src="call-function.js" id="upgrade-me"></div>
<script type="module">
  import {upgradeElement} from '/dist/amp/main.mjs';
  const worker = await (upgradeElement(document.getElementById('upgrade-me'), '/dist/amp/worker/worker.mjs').)

   const result = await worker.callFunction('getRemoteData');
   console.log(result);
</script>

// worker.js
function getRemoteData() {
  return Promise.resolve({ big: 'tuna' });
}
exportFunction("getRemoteData", getRemoteData)

questions i'd like help with

I noticed we don't specify filesize requirements for amp versions. Is this intentional / should I add them?

todo / next steps

(Future PR) create a "lite" build for those that don't need to interact with DOM.
support passing arguments
add tests

cc @choumx @jridgewell

dreamofabear

Bikeshed: "Call function"? :)

src/main-thread/worker.ts

src/worker-thread/function.ts

samouri · 2020-05-05T18:17:32Z

To summarize some of the discussion from late last week with @choumx and @jridgewell, here are the next steps:

Change the interface of worker-dom such that the API returns a new custom object with just callFunction and terminate as opposed to the standard Worker interface.
Bump version
TBD what to do about workers declaring exported functions.

src/main-thread/commands/function.ts

package.json

dreamofabear · 2020-05-18T19:47:00Z

src/main-thread/commands/function.ts

+
+  const index = fnCallCount;
+  promiseMap[index] = { resolve, reject, promise };
+  return { promise, index: fnCallCount++ };


Do we need to worry about overflow?

I don't think so. The fnCallCount is only used to correlate fn call requests and its associated response. Reusing an older index is fine as long as the reused index had actually completed.

It would technically be a bug if there were some super super long running task with index=0 and then while it were running, we launched Number.MAX_VALUE other calls until overflow. And then the next call would overwrite the first one.

Seems okay to me?

TIL js doesn't overflow integers! It rounds to either Number.MAX_VALUE or to Infinity.

For example, Number.MAX_VALUE + 1 === Number.MAX_VALUE. For our use case I'd prefer it wrapped, but I also really dont see anyone hitting 9 quadrillion callFunction requests

Me neither but should be easy to wrap to 0 when we hit MAX_VALUE.

src/main-thread/commands/function.ts

src/main-thread/exported-worker.ts

src/main-thread/configuration.ts

morsssss · 2020-05-18T21:16:55Z

Coolness

/cc @AndrewKGuan @alankent

dreamofabear · 2020-05-22T01:14:40Z

src/main-thread/commands/function.ts

+        if (status === ResolveOrReject.RESOLVE) {
+          promiseMap[index].resolve(JSON.parse(strings.get(value)));
+        } else {
+          promiseMap[index].reject(JSON.parse(strings.get(value)));


How is undefined return value handled here? Are there other cases where JSON.parse can fail/throw?

Nit: Hoist the JSON.parse bit.

Answer: it breaks :). Solving it required a bit of a hack since StringContext will return '' in the case of a falsy value. Therefore I added a new hasIndex method so we can distinguish between undefined and the empty string.

I've also added a test case for this.

Did you push changes yet?

src/test/main-thread/commands/function.test.ts

src/worker-thread/function.ts

samouri · 2020-06-01T14:07:39Z

@choumx: can you merge? I don't have access

kristoferbaxter · 2020-06-01T15:49:05Z

Added you to the write permission group @samouri.

kristoferbaxter · 2020-06-01T15:49:58Z

demo/call-function/index.html

+        worker
+          .callFunction("tooCoolToExist")
+          .catch((err) => console.error(err));
+        worker


Can you add an await example to this demo?

sure thing!

kristoferbaxter · 2020-06-01T15:50:47Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "@ampproject/worker-dom",
-  "version": "0.24.0",
+  "version": "0.25.0",


Do you need to manually update the version?

how can I automatically update the version?

The release script will do this automatically (check out RELEASING.md).

kristoferbaxter · 2020-06-01T15:52:06Z

src/main-thread/commands/function.ts

+    reject = rej;
+  });
+
+  // Wraparound to 0 in case someone attempts to register over 9 quadrillion promises.


LOL. This is likely not necessary, but funny nonetheless.

kristoferbaxter · 2020-06-01T16:07:18Z

src/worker-thread/function.ts

+      TransferrableMutationType.FUNCTION_CALL,
+      ResolveOrReject.REJECT,
+      index,
+      store(JSON.stringify(`[worker-dom]: Exported function "${fnIdentifier}" could not be found.`)),


Is there a reason we need to do this? Could we just reject with a generic error?

Isn't it more helpful to give the devs a useful error message?

Discussed this in Chat. The current method has higher DevEx whereas not having a helpful error message would save bytes.

We can have the best of both worlds by eventually having two builds, one for dev and one for prod. For now I'd like to merge with the slightly higher DevEx option esp as we work on getting to the optimal solution.

samouri · 2020-06-05T17:29:15Z

I'm going to hold off on merging this until I land a fix for #872.

Edit: nevermind. My gut says there is something wrong with my local setup since this built fine. Merging now

googlebot added the cla: yes label Apr 23, 2020

samouri marked this pull request as ready for review April 24, 2020 18:10

dreamofabear reviewed Apr 26, 2020

View reviewed changes

src/main-thread/worker.ts Outdated Show resolved Hide resolved

src/worker-thread/function.ts Outdated Show resolved Hide resolved

samouri changed the title ~~Support invokeFunction API~~ Support callFunction API Apr 28, 2020

samouri added 6 commits May 5, 2020 15:48

Support FunctionInvocations from the main-thread.

e37e8bf

works e2e

21e3da2

edit filesize again

72cd523

support passing arguments

3a6e20f

fix comment

2547c67

rename to callFunction and create ExportedWorker

520d143

samouri force-pushed the exec-fn branch from 19cf134 to 520d143 Compare May 5, 2020 19:55

revert whitespace changes

ec3c674

samouri mentioned this pull request May 5, 2020

amp-script: catch user errors in worker scripts ampproject/amphtml#28207

Merged

samouri added 4 commits May 6, 2020 15:29

add tests

07ce232

fix bugs due to module state. not good very bad

6b4af3f

add main-thread tests

b1a0596

fix tests

f95383f

dreamofabear reviewed May 18, 2020

View reviewed changes

samouri added 3 commits May 19, 2020 12:21

address feedback

4d07722

Merge branch 'master' into exec-fn

926f295

selfcleaning cycle

11d7a50

dreamofabear reviewed May 22, 2020

View reviewed changes

jridgewell mentioned this pull request May 26, 2020

wg-runtime Status Update 2020-05-26 ampproject/wg-runtime#36

Closed

handle an undefined return

d5405ad

dreamofabear approved these changes May 29, 2020

View reviewed changes

increase size of main.mjs

7070959

kristoferbaxter approved these changes Jun 1, 2020

View reviewed changes

samouri added 2 commits June 2, 2020 12:22

address kbax comments

382523d

Merge branch 'master' into exec-fn

078a66e

samouri merged commit 2bf0a5b into ampproject:master Jun 5, 2020

samouri deleted the exec-fn branch June 5, 2020 19:48

samouri mentioned this pull request Sep 30, 2020

I2I: Run 3p service scripts in AMP ampproject/amphtml#30193

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support callFunction API #850

Support callFunction API #850

samouri commented Apr 23, 2020 •

edited

Loading

dreamofabear left a comment

samouri commented May 5, 2020

dreamofabear May 18, 2020

samouri May 19, 2020

samouri May 19, 2020 •

edited

Loading

dreamofabear May 22, 2020

samouri May 27, 2020

morsssss commented May 18, 2020

dreamofabear May 22, 2020

samouri May 27, 2020 •

edited

Loading

dreamofabear May 28, 2020

samouri May 29, 2020 •

edited

Loading

samouri commented Jun 1, 2020 •

edited

Loading

kristoferbaxter commented Jun 1, 2020

kristoferbaxter Jun 1, 2020

samouri Jun 2, 2020

kristoferbaxter Jun 1, 2020

samouri Jun 1, 2020

kristoferbaxter Jun 1, 2020

kristoferbaxter Jun 1, 2020

kristoferbaxter Jun 1, 2020

samouri Jun 2, 2020

samouri Jun 5, 2020

samouri commented Jun 5, 2020 •

edited

Loading

Support callFunction API #850

Support callFunction API #850

Conversation

samouri commented Apr 23, 2020 • edited Loading

dreamofabear left a comment

Choose a reason for hiding this comment

samouri commented May 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouri May 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morsssss commented May 18, 2020

Choose a reason for hiding this comment

samouri May 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouri May 29, 2020 • edited Loading

Choose a reason for hiding this comment

samouri commented Jun 1, 2020 • edited Loading

kristoferbaxter commented Jun 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouri commented Jun 5, 2020 • edited Loading

samouri commented Apr 23, 2020 •

edited

Loading

samouri May 19, 2020 •

edited

Loading

samouri May 27, 2020 •

edited

Loading

samouri May 29, 2020 •

edited

Loading

samouri commented Jun 1, 2020 •

edited

Loading

samouri commented Jun 5, 2020 •

edited

Loading