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

[KLarpen]

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

impress/test/config/server.js

Lines 25 to 28 in fa4d0bb

	workers: {
	pool: 2,
	wait: 2000,
	timeout: 5000,

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,
  },
...
});

3. 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;
  },
});

4. Check that file application/lib/invoke1/method1.js contains line await metarhia.metautil.delay(2000);
5. Run server
6. Open browser tab with DevTools (console & network tabs in it)
7. Check that WS request api is active and there is messages
8. Type in console await api.example.testTimeout(); and run
9. 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:

• use of workers.wait setting in

  impress/impress.js

  Line 166 in fa4d0bb

  const pool = new Pool({ timeout: workers.wait });

• workers.timeout didn't mentioned anywere in impress codebase
• invoke message handler in worker.js don't have any timeout applied to handler function call

  impress/lib/worker.js

  Lines 51 to 69 in fa4d0bb

  	invoke: async ({ exclusive, data, port }) => {
  	const { method, args } = data;
  	const { sandbox } = application;
  	const handler = metarhia.metautil.namespaceByPath(sandbox, method);
  	if (!handler) {
  	const error = new Error('Handler not found');
  	return void port.postMessage({ name: 'error', error });
  	}
  	const msg = { name: 'invoke', status: 'done' };
  	try {
  	const result = await handler(args);
  	port.postMessage({ ...msg, data: result });
  	} catch (error) {
  	port.postMessage({ name: 'error', error });
  	application.console.error(error.stack);
  	} finally {
  	if (exclusive) parentPort.postMessage(msg);
  	}
  	},

• Planner.execute also don't apply any timeout on execution of a task itself. Possibly "Task execution timeout" scheduler.timeout setting also doesn't work.

  impress/lib/planner.js

  Lines 159 to 175 in fa4d0bb

  	async execute(task, once = false) {
  	if (task.executing) return void this.fail(task, 'Already started task');
  	task.lastStart = Date.now();
  	task.executing = true;
  	try {
  	await this.enter(task.name);
  	task.result = await this.invoke(task);
  	} catch (error) {
  	task.error = error;
  	if (error.message === 'Semaphore timeout') {
  	this.fail(task, 'Scheduler queue is full');
  	} else {
  	this.console.error(error.stack);
  	}
  	} finally {
  	this.leave(task.name);
  	}

[KLarpen]

Initially the wait and timeout properties of the workers settings were introduced by #1664 . That PR adds actual usage of workers.wait in Pool but not timeout.