fix: Log uncaught exceptions in worker thread handlers

[aldenquimby]

Motivation and Context

• Fixes Uncaught exception from worker thread should be logged to console #1541
• First attempt at Log uncaught exceptions in worker thread handlers #1543 did not work b/c logs from the worker thread do not go to the console
  • alternative considered was to add log here, but the error has been swallowed already at that point, so that won't work either:

    serverless-offline/src/lambda/handler-runner/worker-thread-runner/WorkerThreadRunner.js

    Lines 40 to 43 in 0f6c9f4

    	if (value instanceof Error) {
    	rej(value)
    	return
    	}

• Cleanest way to solve this appears to be just removing try/catch (what this PR proposes), so root error gets logged here:

  serverless-offline/src/events/http/HttpServer.js

  Lines 615 to 619 in 0f6c9f4

  	log.error(errorMessage)
  	if (!this.#options.hideStackTraces) {
  	log.error(err.stack)
  	}

How Has This Been Tested?

• npm run lint passes locally
• npm run test:unit - cannot get this to work locally. jest dependency appears to be missing?
• Locally after this change, per the bug report, I now see:

✖ Example failure
✖ Error: Example failure
      at handler (/Users/alden/.../appHandler.js:27:11)
      at InProcessRunner.run (file:///Users/alden/.../node_modules/serverless-offline/src/lambda/handler-runner/in-process-runner/InProcessRunner.js:87:20)
      at async MessagePort.<anonymous> (file:///Users/alden/.../node_modules/serverless-offline/src/lambda/handler-runner/worker-thread-runner/workerThreadHelper.js:25:14)

• So I get both the underlying error message and the correct stack trace

[dnalborczyk]

thank you @aldenquimby !

regarding your first attempt, that might be a problem with logging and worker threads: nodejs/node#30491

I think it might be nice to see which handler (function key) actually caused the problem. I'll have a look at your first attempt and the log not showing up.

[aldenquimby]

@dnalborczyk ok great, thank you. Another possible solution if you want to ensure the function name shows:

    } catch (e) {
      throw new RethrownError(`Uncaught error in '${this.#functionKey}' handler.`, e)
    }

And we'd need to create RethrownError:

class RethrownError extends Error {
  constructor(message, error) {
    super(message)
    this.name = this.constructor.name
    this.stack = error.stack
  }
}

That seems cleaner to me because then there's just one place errors are logged (in HttpServer.js), vs having console.error scattered throughout

Thoughts on this approach?

[dnalborczyk]

@aldenquimby I try to find some time over the weekend to have a closer look. I like your ideas, I just want to see what exactly is wrong. I don't know crazy much about worker threads and the usage in the plugin is pretty much my first and only attempt.

just wanted to mention a couple things as food for thought:

serverless-offline/src/lambda/handler-runner/in-process-runner/InProcessRunner.js

Line 92 in 07cdd04

result = handler(event, lambdaContext, callback)

the handler call you are referring to would only be caught in he catch block if the handler is a synchronous function and some unexpected unhandled error would occur, which would be only the case if someone would use callbacks or the deprecated context methods. the catch block would only be activated by accident, as synchronous handlers are not supported.

the second handler call

serverless-offline/src/lambda/handler-runner/in-process-runner/InProcessRunner.js

Line 109 in 07cdd04

const callbackResult = await Promise.race(callbacks)

does not have a try/catch block, but probably should. this is for all asynchronous stuff (async function, returned Promise, callback (Promise wrapped), as well as context methods (Promise wrapped)).

[dnalborczyk]

@aldenquimby I pulled in your PR and added this: 06d348d . does this still cover your expectations?

one of the problems was that the serverless/utils logger did not seem to work in a worker thread, but console.log did. (I didn't feel like digging into it).

[aldenquimby]

@dnalborczyk yes I just upgraded to v9.2.6 and error logs are working for me now, thank you!