feat: add binary output port on HttpCallNode

[liaujianjie]

Resolves #386

Context

Previously, it wasn't possible to make a HTTP call (via HttpCallNode) and have the binary/blob data passed to an ImageNode. This PR introduces a new binary output port (alongside the existing body and json output ports) on HttpCallNode to make this possible.

Screengrab

How to test

1. Launch the app with the test Rivet project below
2. Switch executor to Node
3. Run entire graph
4. Observe that fetched image (of my cat 😼) should show up in the ImageNode, proving that the feature works

Test Rivet project

version: 4
data:
  attachedData:
    trivet:
      testSuites: []
      version: 1
  graphs:
    q8Tc5Q-34WB8e-loWqoON:
      metadata:
        description: ""
        id: q8Tc5Q-34WB8e-loWqoON
        name: Untitled Graph
      nodes:
        '[Ux7TlwMXhLmKxUjdiDuIc]:httpCall "Http Call"':
          data:
            body: ""
            errorOnNon200: true
            headers: ""
            method: GET
            url: https://github.com/liaujianjie.png
          outgoingConnections:
            - binary->"Image" owgnZUkpDj7Ik93OtyOGd/data
          visualData: 497/390/280/3//
        '[owgnZUkpDj7Ik93OtyOGd]:image "Image"':
          data:
            mediaType: image/png
            useDataInput: true
            useMediaTypeInput: false
          visualData: 1029/376/484.1567767955994/7//
  metadata:
    description: ""
    id: _A_qCLGKHitAnTSshRby2
    title: Untitled Project
  plugins: []

[liaujianjie]

This fixes an issue where hovering on the new binary output port causes the app to crash:

It crashes because value.value is a POJO instead of a Uint8Array. For example, Uint8Array(2) [123, 45] would end up being { "0": 123, "1": 45 } here.

I suspect this is because Uint8Array is serialised as such a POJO when stringified with JSON.stringify()—as is required when passing the data over to the Node.js executor. This can be verified with:

const binaryOnBrowser = new Uint8Array([123,45]); // Uint8Array(2) [123, 45]
const binaryInTransit = JSON.stringify(binaryOnBrowser);
const binaryOnNode = JSON.parse(binaryInTransit); // { "0": 123, "1": 45 }

There's probably a better place to put this coercion fix but I'm not sure where. 😬 Need the help of the maintainers on this one.

[abrenneke]

Yeah, binary values don't get transmitted right across the remote debugger protocol. Definitely something that needs to be fixed at some point.

[abrenneke]

This is great @liaujianjie! I was thinking, what if there were a setting toggle in the node instead of both binary and text outputs? I can't really think of a case where you'd want both the binary and text output of an HTTP call, so instead we can just let the use choose which type the output of the node should be in the settings panel. That makes the node maybe simpler to use?

[liaujianjie]

This is great @liaujianjie! I was thinking, what if there were a setting toggle in the node instead of both binary and text outputs? I can't really think of a case where you'd want both the binary and text output of an HTTP call, so instead we can just let the use choose which type the output of the node should be in the settings panel. That makes the node maybe simpler to use?

Definitely agree with this approach, thought I'd need some pointers on how I should do that since I'm still pretty new to Rivet.

An alternative would be to separate these into 2 PRs? So we have this PR for adding support for for the binary port on HttpCallNode, then a second PR for adding the switch/select to choose between JSON/binary/text. This way we can also keep the scope of this PR small.

Thoughts?

[abrenneke]

That would mean maybe two releases and backwards incompatible stuff, so probably better to nail this the first time. Should be pretty easy, just checking this.data.isBinary in both the output definitions function and the main process body. No more difficult than the stuff you've already written.

[liaujianjie]

What about JSON outputs? Shouldn't the output type be one of:

1. string,
2. JSON, or
3. binary?

[liaujianjie]

@abrenneke I've went ahead to implement it such that the user can either switch between:

1. binary
2. string or JSON

The option to select between the two is just below the body:

What do you think?

[abrenneke]

Looks great, thank you!

[abrenneke]

@all-contributors please add @liaujianjie for code

[allcontributors]

@abrenneke

I've put up a pull request to add @liaujianjie! 🎉