nrkno / sofie-emberplus-connection

Ember+ Connection: A Part of the Sofie TV Studio Automation System
https://github.com/nrkno/Sofie-TV-automation/
MIT License
20 stars 15 forks source link

Getting timeout when expanding #21

Open boristian opened 2 years ago

boristian commented 2 years ago

Hi, first of all, thank you for this rewrite 👍

When playing around with it i'm always getting UnhandledPromiseRejectionWarning: Error: Request timed out when trying to expand a tree.

Here is my code (more or less copied from the README.md):

const { EmberClient } = require('emberplus-connection')

const host = '10.0.0.10'
const port = 9000

const run = async () => {
  const client = new EmberClient(host, port)

  client.on('error', err => console.log(err))

  await client.connect()

  // Get Root info
  const req = await client.getDirectory(client.tree)
  await req.response

  // Get a Specific Node
  const node = await client.getElementByPath('1.1.1.1.1')
  console.log(node)

  // Expand entire tree under node 0
  await client.expand(client.tree)

  console.log(client.tree)
}

run ()

Getting a specific path is no problem, but the expand. I have tried with TinyEmberPlus from Lawo and a DHD device, always the same. Am I doing something wrong?

Greetings and thank you in advance

boristian commented 2 years ago

Never mind, here is what worked for me:

const { EmberClient } = require('emberplus-connection')

const host = '10.0.0.10'
const port = 9000

const run = async () => {
  const client = new EmberClient(host, port)

  client.on('error', err => console.log(err))

  await client.connect()

  // Get Root info
  const req = await client.getDirectory(client.tree)
  const root = await req.response

  // Get a Specific Node
  const node = await client.getElementByPath('1.1.1.1.1')
  console.log(node)

  // Expand entire tree under root
  await client.expand(root)

  console.log(root)
}

run ()
dewiweb commented 2 years ago

Same problem here:

Unhandled Exception UnhandledRejection Error: Request timed out
    at C:\Users\Administrateur\Documents\GitHub\MCxOSCnext\node_modules\emberplus-connection\dist\Ember\Client\index.js:525:32
    at Map.forEach (<anonymous>)
    at EmberClient._resendTimer (C:\Users\Administrateur\Documents\GitHub\MCxOSCnext\node_modules\emberplus-connection\dist\Ember\Client\index.js:512:28)   
    at Timeout._onTimeout (C:\Users\Administrateur\Documents\GitHub\MCxOSCnext\node_modules\emberplus-connection\dist\Ember\Client\index.js:47:46)
    at listOnTimeout (node:internal/timers:559:17)
    at process.processTimers (node:internal/timers:502:7)
11:41:38 / (node:4492) UnhandledPromiseRejectionWarning: UnhandledRejection Error: Request timed out
    at C:\Users\Administrateur\Documents\GitHub\MCxOSCnext\node_modules\emberplus-connection\dist\Ember\Client\index.js:525:32
    at Map.forEach (<anonymous>)
    at EmberClient._resendTimer (C:\Users\Administrateur\Documents\GitHub\MCxOSCnext\node_modules\emberplus-connection\dist\Ember\Client\index.js:512:28)   
    at Timeout._onTimeout (C:\Users\Administrateur\Documents\GitHub\MCxOSCnext\node_modules\emberplus-connection\dist\Ember\Client\index.js:47:46)
    at listOnTimeout (node:internal/timers:559:17)
    at process.processTimers (node:internal/timers:502:7)
(Use `electron --trace-warnings ...` to show where the warning was created)
11:41:38 / (node:4492) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)

with this function:

async function expandtree(){
        const request = await eGet.getDirectory(eGet.tree)
        const allroot = await request.response
        const expanded = await eGet.expand(allroot)
        console.log("EXPANDED:",expanded)
      }
      expandtree()
boristian commented 7 months ago

Hi there, i reopen this issue because i did some further testing. I think the timeout comes from nodes without children and the resulting response for that node from TinyEmber or the DHD device.

I built the following tree on TinyEmber and with the provider from this library:

EmberViewer

For both providers i made getDirectory request for path 1.2 wich is NodeWithoutChildren.

For the emberplus-connection everything is OK and the response package looks like this:

Wireshark_emberplus-connection

I printed the argument incoming and the resulting changes for _handleIncoming to the console:

{
  value: {
    '0': QualifiedElementImpl {
      contents: [EmberNodeImpl],
      children: {},
      parent: undefined,
      path: '1.2'
    }
  },
  errors: []
}
[
  {
    path: '1.2',
    node: NumberedTreeNodeImpl {
      contents: [EmberNodeImpl],
      children: {},
      parent: [QualifiedElementImpl],
      number: 2
    }
  },
  {
    path: '1.2',
    node: NumberedTreeNodeImpl {
      contents: [EmberNodeImpl],
      children: {},
      parent: [QualifiedElementImpl],
      number: 2
    }
  }
]

For the TinyEmberplus provider it looks like this:

Wireshark_TinyEmber

{
  value: {
    '0': QualifiedElementImpl {
      contents: [EmberNodeImpl],
      children: undefined,
      parent: undefined,
      path: '1.2'
    }
  },
  errors: []
}
[
  {
    path: '1.2',
    node: {
      contents: [EmberNodeImpl],
      children: undefined,
      parent: [NumberedTreeNodeImpl],
      path: '1.2',
      number: 2
    }
  }
]

i am not sure but i think the reason for the timeout is that the TinyEmber provider does not send any contents or information about the any children. Any ideas? I don't know if it is "wrong" what TinyEmber is doing, but the DHD has the same behaviour. So there are devices around which do answer like that and i think the library should be able to handle it. For nodes with children (1.1, NodeWithChildren) getDirectory is OK for both providers.

boristian commented 3 months ago

Hi, i looked into the Ember+ protocol specification (Version 2.50) and on page 31:

When a consumer performs a GetDirectory request on a node without any children, the node must report itself without any properties set (even the identifier has to be omitted). Otherwise it is not possible to distinguish it from an ordinary update. That way, the consumer knows that a query has been performed on this node and that it has no child elements.

So i think this is what TinyEmber and the DHD-device is doing and since it is described in the specification, the library should be able to handle this behaviour.

Is _applyRootToTree the place where this should be handled?

It also means that the provider implementation in the lib does not comply with the specification.

mint-dewit commented 2 months ago

Interesting, I may have missed that when implementing the Provider yeah. Would have to be updated.

What is odd about the issue with the Client implementation is that we do get the Change with the correct path. This means that the request for the getDirectory should be resolved just fine. The canBeExpanded function on line 311 could probably be updated but that doesn't explain why it would time out, I wonder if the issue here is that the provider stops responding to additional queries to the node without children.

boristian commented 1 month ago

The request times out because of the _handleIncoming method. Since the change has no children, the for...of loop continues and does not delete the request. Setting the node's children property to {} when update.contents.identifier is undefined gets around this.

https://github.com/boristian/sofie-emberplus-connection/blob/FIX_getDirectory-timeout-errors/src/Ember/Client/index.ts

https://github.com/nrkno/sofie-emberplus-connection/compare/master...boristian:sofie-emberplus-connection:FIX_getDirectory-timeout-errors

Is this the way to go and worth making a PR?

mint-dewit commented 3 weeks ago

I think #39 is getting at a very similar issue and it would be better to not expect children for a getDirectory at all then. Instead we should handle that case in the _handleIncoming method

boristian commented 3 weeks ago

Hi, thanks for your response. Not expecting an element to have children is correct. This is why i changed ExpectResponse.HasChildren to CanHaveChildren. I thought, handling the type-specific logic in _updateTree might be good. At first, I wanted to use _updateEmberNode, but you cannot set the children prop there because it's not in the EmberNode model.

mint-dewit commented 3 days ago

@boristian I added some changes in https://github.com/nrkno/sofie-emberplus-connection/pull/40 and can confirm that the library handles nodes (and parameters) without children set correctly in that branch. In addition I fixed some issue in the expand that meant it could only be used without having sent a GetDirectory to the root of the tree before