feat(graphql): Improve GraphQL Yoga fatal exception handling

[will-ks]

Currently the GraphQL Yoga initialization code is wrapped in two separate try catch blocks.

In the second block, exceptions are not currently logged, and if an exception is caught here, the GraphQL server fatally crashes but you get no useful logging- you just get this:

[0] api | /Users/willks/projects/redwood-saas-starter/node_modules/pino/lib/tools.js:59
[0] api |       if (typeof this[msgPrefixSym] === 'string' && msg !== undefined && msg !== null) {
[0] api |                      ^
[0] api | 
[0] api | TypeError: Cannot read properties of undefined (reading 'Symbol(pino.msgPrefix)')
[0] api |     at LOG (/Users/willks/projects/redwood-saas-starter/node_modules/pino/lib/tools.js:59:22)
[0] api |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
[0] api | 

It doesn't seem to be useful to separate the initialization code into two try catches and treat them differently, as exceptions in either block are fatal, so this PR combines the two blocks. Now you get proper pino logging for exceptions wherever they occur.

Also, when I was initially trying to debug this, before looking into redwood's code, I thought about using the onException callback option in GraphQLYogaOptions, but it's not all that useful as it doesn't pass through the original exception. So this PR also changes that to pass through the original exception.

[Tobbe]

Thanks for the PR @will-ks! I'll ask @dthyresson to take a look when he has a few minutes to spare 🙂

[dthyresson]

Ah, that's some very original createYoga code I think :) @will-ks and I'll have to see if there was a reason we did that ... though it may have been an oversight and if so, this PR is great to correct that! I'll review and let know.

[will-ks]

Hi @dthyresson Sure, I don't have the full picture of the process lifecycle, so definitely if it's more appropriate to just log there I can change the PR to just do that.

So - how about we just add a console.error() in

Would it not be better to use logger? I've force pushed the branch to use that.

[dthyresson]

Hey @will-ks sorry we have been busy on some other features and I apologies for the late reply. I know seems a bit silly maybe to iterate on such a small Pr but is there a way I can reproduce this in a new project? I am tying to think of a way or place that the exception would be logged like you had in your description.

Looks like something with Pino logger?

I just want to reproduce the case to see what it looks like and then merge.

Thanx for understanding.

[peterp]

Hey there, I'm cleaning up this repository to make it slightly easier to see movement. We're making big changes to everything we're doing, so just know that I appreciate the effort you put in here, and hope to get you helping out again.