Skip to content
This repository was archived by the owner on Dec 19, 2023. It is now read-only.

Conversation

Sam-Kruglov
Copy link

related to #305 but it has become stale.
@oliemansm could you, please, take a look?

@Sam-Kruglov Sam-Kruglov force-pushed the bug/@ExceptionHandler_registration branch from 3519d13 to 76ae8c8 Compare January 2, 2020 09:28
@Sam-Kruglov
Copy link
Author

GraphQLErrorHandlerTest failed

@Sam-Kruglov Sam-Kruglov force-pushed the bug/@ExceptionHandler_registration branch from 76ae8c8 to 55510a3 Compare January 2, 2020 10:20
@Sam-Kruglov
Copy link
Author

Not sure if getContext().publishEvent(new ApplicationReadyEvent(new SpringApplication(), new String[0], getContext())); the best way but manually starting up spring context is also not the best way, so, at least it fits the rest of the code.
Please, consider using ApplicationContextRunner

@Sam-Kruglov
Copy link
Author

Sam-Kruglov commented Jan 2, 2020

May I also propose that GraphQLErrorHandlerFactory does not deal with DefaultGraphQLErrorHandler? So that it is more flexible for the client to configure the default one.
Maybe return List<GraphQLErrorFactory>

@Sam-Kruglov
Copy link
Author

Another suggestion: make GraphQLErrorFromExceptionHandler and GraphQLErrorFactory public so that I can change the fallback values. E.g. I always want an error code in extensions, so I would change GenericGraphQLError and ThrowableGraphQLError at least

@oliemansm
Copy link
Member

@Sam-Kruglov Just merged the other PR dealing with the same issue. Is there anything in this PR still relevant or has this now become obsolete?

@Sam-Kruglov
Copy link
Author

Obsolete now. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants