search

digikare / nestjs-prom

A prometheus module for nestjs
160 stars 54 forks source link

PromCatchAllExceptionsFilter not compatible with GraphQL request #47

Open micolator opened 3 years ago

micolator commented 3 years ago

A gql request with an exception give this exception : TypeError: Cannot read property 'baseUrl' of undefined", " at PromCatchAllExceptionsFilter.catch

Some info to use gql context : https://docs.nestjs.com/graphql/other-features#exception-filters https://docs.nestjs.com/fundamentals/execution-context#current-application-context

spike008t commented 3 years ago

On which version are you using?

Yes that's right, my module is only for http at the moment. I'll try to make it works with gql.

micolator commented 3 years ago

I use v1.0.0 Thanks for your feed back, and great work !

lukasender commented 3 years ago

Is there a workaround for this? I'm getting the same error.

Thanks for your help in advance! Great work! I'd love to keep using nestjs-prom! :)

chasevida commented 3 years ago

@lukasender I haven't completed a full integration with this module but I made some changes you can see here (https://github.com/chasevida/nestjs-prom/blob/feat/graphql/lib/prom-catch-all.exception-filter.ts) that enable integration with GraphQL.

Essentially, you have to check for the context and switch appropriately - in this case to a GQL Context. However, there may be some issues depending on how you setup you Nestjs GraphQL module. Also the Nestjs BaseExceptionFilter integration is a little bit more nuanced and I haven't sorted out how this may work together so I've removed that in the case of a GraphQL request which may not be ideal depending on the rest of your setup.

Happy to turn this into a PR if there's any guidance on what else should be considered here.

kodeine commented 3 years ago

Any updates on this?

kodeine commented 3 years ago

@chasevida can you please submit PR for this

issackr commented 3 years ago

+1

roderik commented 3 years ago

I needed it as well since it is swallowing up the real errors underneath, so i made a fork, did the code change by @chasevida, upgraded all dependencies to their latest version (most notably nestjs to v8) and committed the build artefacts. You can use it like this until this change is integrated in the main repo.

"@digikare/nestjs-prom": "roderik/nestjs-prom#master",

YMMV...

chasevida commented 3 years ago

Hi @kodeine, unfortunately I haven't had the time to come back around to this. I think @roderik may have a more up to date workaround. There's still a few things I would need to finalise and get direction on before I think this would be PR worthy. Specifically I need to come back and look at how BaseExceptionFilter works and check the integration for that is compatible. Currently, I've removed it for GQL requests which is quite an opinionated decision that may or may not suit you - I need to check more on this.