metarhia / impress

Enterprise application server for Node.js and Metarhia private cloud ⚡
https://metarhia.com
MIT License
966 stars 128 forks source link

Configuration server.worker.timeout doesn't limit task execution time #1950

Closed KLarpen closed 9 months ago

KLarpen commented 9 months ago

Impress and Node.js versions

Impress: 3.0.13, Metautil ^3.15.0, Node: 20.9.0

Platform

macOS 14.1.2 Darwin Kernel Version 23.1.0: Mon Oct 9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 arm64

Describe the bug

Configuration file config/server.js expected from an application folder contains property worker.timeout https://github.com/metarhia/impress/blob/fa4d0bbb9029ffac4dfa65acf7cbe8cd3cd8dae8/test/config/server.js#L25-L28

The documentation describes its purpose as "Worker task execution timeout". However the value of config.server.worker.timeout actually doesn't apply that behavior.

To Reproduce

  1. Start with Example project
  2. Change the config at application/config/server.js to contain
({
...
  workers: {
    pool: 1,
    wait: 1000,
    timeout: 300,
  },
...
});
  1. Put file application/api/example/testTimeout.js
({
  access: 'public',
  async method() {
    const res = await application.invoke({
      method: 'lib.invoke1.method1',
      args: { exclusive: 'invoked' },
      exclusive: true,
    });
    return res;
  },
});
  1. Check that file application/lib/invoke1/method1.js contains line await metarhia.metautil.delay(2000);
  2. Run server
  3. Open browser tab with DevTools (console & network tabs in it)
  4. Check that WS request api is active and there is messages
  5. Type in console await api.example.testTimeout(); and run
  6. You will receive successful answer after 2 seconds
{"type":"call","id":4,"method":"example/testTimeout","args":{}}
{"type":"callback","id":4,"result":{"hello":"world"}}

The server log contains nothing special: just log of the successful request to example/testTimeout.

Expected behavior

The server configuration worker.timeout setting should be applied globally to limit execution time of worker task. By the way default value 5000 in Example project should be rised to at least 60000 in my opinion because worker tasks might be long running background processes. The request from example expected to be failed after about 300 milliseconds, but I don't think that proper error for this case will be 408. Maybe 500 "Internal Server Error" is more appropriate.

Screenshots

No response

Additional context

Currently I'm investigating possible fix to make pull request with it. Anyway I'm open to receive proposals from other contributors. I will appreciate an information about history of this feature implementation. As of now I had found:

KLarpen commented 9 months ago

Initially the wait and timeout properties of the workers settings were introduced by https://github.com/metarhia/impress/pull/1664 . That PR adds actual usage of workers.wait in Pool but not timeout.