Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Errors when Serializing to JSON #4160

Closed
capesean opened this issue Feb 25, 2016 · 15 comments
Closed

Errors when Serializing to JSON #4160

capesean opened this issue Feb 25, 2016 · 15 comments

Comments

@capesean
Copy link

I am hitting an error that I can't quite pinpoint. If you can tell me how to debug it better I'll gladly provide the details. This was working fine with EF7 before the change to EntityFrameworkCore 1.0, but now I'm getting these two types of errors. I don't think it's an EF issue because the data gets returned from the database fine, it seems to error when it gets serialized in the WebApi. Although maybe it's an issue with Json.Net.

I have an EF model of clients that have projects. A project has a Project Manager, a Project Type, (etc) as related entities.

When I query the API endpoint to list the projects, I run this code:

return Ok(
    DbContext.Projects
        .Include(p => p.Client)
        .Include(p => p.ProjectManager)
        .Include(p => p.ProjectType)
    );

I get the error:

HTTP Error 502.3 - Bad Gateway
The specified CGI application encountered an error and the server terminated the process.

Most likely causes:
The CGI application did not return a valid set of HTTP errors.
A server acting as a proxy or gateway was unable to process the request due to an error in a parent gateway.

If I remove the line .Include(p => p.Client) then I get a different issue: the json gets truncated after about 259 projects. If I put a .Take(258) in there, then it works fine.

It looks like 2 different issues - the CGI error and the truncating error. As mentioned, this worked a few days ago before I upgraded to AspNetCore and EntityFrameworkCore.

I'm not sure how to get more error information out but if direct me I'll do it.

@capesean
Copy link
Author

This appears to be an issue with Entity Framework.

@capesean
Copy link
Author

It's not entirely an EF issue. Yes, there is a circular reference issue in EF, but if I do JsonConvert.SerializeObject on the EF result and then send that string back, it works fine. But if I just do Ok(queryresults) then the result is truncated.

@gdoron
Copy link

gdoron commented Feb 26, 2016

I had something similar, try setting this in the Configuration method in the Startup class:

services.AddMvc().AddJsonOptions(options => {
                options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore;
            });

@gdoron
Copy link

gdoron commented Feb 26, 2016

@Eilon shouldn't the default needs to be changed?
Since if the default value comes into play, it will throw an exception...

@Eilon
Copy link
Member

Eilon commented Feb 26, 2016

I think the default value of not ignoring reference loops is correct because you want to avoid serializing things that have loops to begin with. Changing the default would likely mask problems with the data being serialized, no?

@JamesNK
Copy link
Member

JamesNK commented Feb 27, 2016

Erroring when a loop is encountered is standard behavour for serializers.

Options are there to fix the error (including updating the serialized object). I don't think you should pick one for devs.

@OlegKi
Copy link

OlegKi commented Feb 27, 2016

@Eilon: I find changing of default for the best way. The usage of .Include is the standard way which suggest Entity Framework. The most people just follow the way and then spend a lot of his time in finding the origin of "HTTP Error 502.3 - Bad Gateway" error.

@JamesNK : I could agree with you if the initial model were oriented on serialization. The developer want just include some field which could be get by INNER JOIN and so he uses .Include. As the result he get "HTTP Error 502.3 - Bad Gateway" error, which gives absolutely no information about the origin of the problem.

Thus, in my personal opinion, changing of defaults would be make Entity Framework more user friendly, especially for newcomers and especially if one unable to report the error with the exact reason on the problem.

@gdoron
Copy link

gdoron commented Feb 27, 2016

@Eilon I agree that if you pass an entity with reference loop to a view you're doing something wrong because you're not using ViewModel/DTO.
But unfortunately from the places I worked at (and from Stackoverflow questions) most developers do not use DTOs, and in matter of fact even the scaffolding provided by MS for creating new Controller do not use DTOs...

@Eilon
Copy link
Member

Eilon commented Mar 7, 2016

Using view models and DTOs is definitely recommended. It's really not a goal for MVC to allow general serialization of EF data models. The reality is that there's effectively a bug in the app, and the bug needs to be fixed, not ignored.

Completely ignoring reference loops is a Very Bad IdeaTM because it just means the serialized data is missing stuff and you can't predict where. That's not good.

@Eilon Eilon closed this as completed Mar 7, 2016
@gdoron
Copy link

gdoron commented Mar 7, 2016

@Eilon I agree, but the scaffolding and templates should be adjusted to use DTOs (AutoMapper is probably a good idea).
And if possible, changing the error message you get if you if you pass a model with a reference loop from HTTP Error 502.3 - Bad Gateway to something a little bit more reasonable, otherwise once ASP.NET Core will go public, it will be the most asked question on Stack Overflow... 😆

@Eilon
Copy link
Member

Eilon commented Mar 7, 2016

@gdoron issues with the templates can be logged here: https://github.com/aspnet/Templates/

However, one thing that's been a constant challenge for us when making the templates is balancing out how easy it is to understand vs. how "real world" something is. Using view models, binding models, and serialization models is definitely more powerful, but it's a large maintenance cost for an app, and more difficult for someone starting to understand (e.g. "how come when I add/change a property in my model I have to do it in 4 places??").

Regarding the Bad Gateway error that's a really tough one: the problem is that you can't know about the error until it's too late, unless you buffer the entire response (which could be several GB or more!).

@datumgeek
Copy link

The fix (ignore loops) worked great for me

my scenario: Web API with EF Core - Returning child data along with parent (via Include)

quite common, i'm guessing...

Ran into the issue described above. Didn't get an error, just no response from the web api.

Happened to stumble upon this thread.

Added the fix (ignore loops).

Worked like a champ !!!

@alelom
Copy link

alelom commented May 22, 2018

I agree with @Eilon, but this bug is still unresolved as to date, and two years have passed.
The issue happens even when literally following Microsoft's own tutorials for ASP Core beginners. This should not happen. I myself, learning ASP from scratch, I have lost several hours to understand what was the issue and then come here. The "fix" (ignore loops) worked for me, but I agree it cannot be regarded as a long term solution.

@Fieel
Copy link

Fieel commented Jul 25, 2018

@gdoron solution is a bad practice because we'd rather avoid circular dependency. I just want my object and its children propreties, simple as that.

Ignoring the circular loop and adding the option in the startup.cs file is not a solution at all, it's just a workaround..

I'm kinda shocked because i'm learning the framework and stumbling upon critical bugs such as this one. As @alexlomba87 noticed, the bug is present and ignored in most .NET Core tutorials and documentation pages. I just lost one entire day at work trying to fix this 👎 and there is no solution

@IssamTechs
Copy link

referencing problem
for example :
employee {
.... ,
customer
}
customer {
.... ,
employee
}
usually when you face certain scenarios of reference problem you can solve it either:
1)adding Json configuration to bypass this error like
services.AddMvc().AddJsonOptions(options => { options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; });
2) make use of view models or DTO (data transfer object) to hold the field that doesn't actually have any referencing problem.

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

No branches or pull requests

9 participants