-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] error renderers for different exception types #93
Conversation
@@ -41,6 +41,14 @@ | |||
<CodeAnalysisIgnoreGeneratedCode>true</CodeAnalysisIgnoreGeneratedCode> | |||
</PropertyGroup> | |||
<ItemGroup> | |||
<Reference Include="EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089, processorArchitecture=MSIL"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We definitely don't want to pull in Entity Framework as a reference. Why do you think we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For thee executions?
What is the status of this PR? |
I think there need to be some decisions around how extensions to Saule happen -- how to determine what renders an Exception, etc. It doesn't make sense to depend on EF in Saule in order to render EF errors, but it should be super straight forward to write on EFErrorRenderer or something like that, ya know? |
I agree, there should be an extension point for this. It should likely be some interface people can implement and pass to Saule using the configuration object (similar to the URL builder). The interface should probably look something like this: public interface ErrorRenderer
{
IEnumerable<JsonApiError> Render(object error);
} where |
I like that. How would saule know to use a custom renderer? |
Same as the URL builder; if the consumer supplies one, Saule will use it. When people build their own, they can derive from a default implementation and only override what they are actually customizing. |
@NullVoxPopuli I'm closing this for now. Feel free to open a new PR with the changes we discussed against the current |
Currently, this doesn't work.
I guess I'm having a hard time figuring out where exactly the exceptions are caught and rendered as json