Register a SA Forums Account here!
JOINING THE SA FORUMS WILL REMOVE THIS BIG AD, THE ANNOYING UNDERLINED ADS, AND STUPID INTERSTITIAL ADS!!!

You can: log in, read the tech support FAQ, or request your lost password. This dumb message (and those ads) will appear on every screen until you register! Get rid of this crap by registering your own SA Forums Account and joining roughly 150,000 Goons, for the one-time price of $9.95! We charge money because it costs us money per month for bills, and since we don't believe in showing ads to our users, we try to make the money back through forum registrations.
 
  • Post
  • Reply
New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

User0015 posted:

Quoting myself here, but converting over to S3 storage was relatively simple.

However, there seems to be a dispose issue between accessing the DB to insert the URL, and the actual file upload to S3 storage. So I've stripped out usings and it's working, but I suspect I've created a memory leak somewhere. Or something worse. Any tips on testing for leaks or 'long lived' connections not releasing?


code:
 public void UploadImages(IFormFileCollection images, ImageType type)
        {
            foreach (var image in images)
            {
                var guid = Guid.NewGuid();
                UploadToDBContext(image, type, guid);
                UploadToS3(image, guid);
            }
        }
Let me know if I've done something insanely stupid here.

That method's not async and you're not awaiting the results. Depending on what your consuming code is doing, that could be a bad thing.

Adbot
ADBOT LOVES YOU

User0015
Nov 24, 2007

Please don't talk about your sexuality unless it serves the ~narrative~!
Awaiting breaks it. I have no idea why.

I also tried to wrap it like this

code:
var tasks = Task.Run( () => {
  UploadToDB...
  UploadToS3...
});

await Task.WhenAll(tasks);
Which also breaks it. In theory, both those calls can run concurrently. I only need to move on after they both finish, because the GUID is what ties them together. I'll try it again just in case it was something else causing the error.

User0015 fucked around with this message at 21:57 on Jan 2, 2019

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

User0015 posted:

Awaiting breaks it. I have no idea why.

I also tried to wrap it like this

code:
var tasks = Task.Run( () => {
  UploadToDB...
  UploadToS3...
});

await Task.WhenAll(tasks);
Which also breaks it. In theory, both those calls can run concurrently. I only need to move on after they both finish, because the GUID is what ties them together.

"Awaiting breaks it" sounds like a big problem.

Your use of Task.Run is potentially a source of the problem. Try this:
code:
var tasks = new [] {
  UploadToDB...
  UploadToS3...
}

await Task.WhenAll(tasks);
Your code was not equivalent. You were creating a new task that created two tasks, then immediately completed. You weren't actually awaiting the results of either.

User0015
Nov 24, 2007

Please don't talk about your sexuality unless it serves the ~narrative~!

New Yorp New Yorp posted:

"Awaiting breaks it" sounds like a big problem.

Your use of Task.Run is potentially a source of the problem. Try this:
code:
var tasks = new [] {
  UploadToDB...
  UploadToS3...
}

await Task.WhenAll(tasks);
Your code was not equivalent. You were creating a new task that created two tasks, then immediately completed. You weren't actually awaiting the results of either.

It seems to be a strange interaction with the await Tasks.WhenAll(..), the foreach loop, and maybe??? Dispose(). It completes one iteration, then when it runs them a second time, it breaks. I might need to solve this in a different way entirely. Probably take a more functional approach instead of iterating through a loop.

edit - The entity framework context has an ObjectDisposedException on the second pass.

User0015 fucked around with this message at 22:40 on Jan 2, 2019

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

User0015 posted:

It seems to be a strange interaction with the await Tasks.WhenAll(..), the foreach loop, and maybe??? Dispose(). It completes one iteration, then when it runs them a second time, it breaks. I might need to solve this in a different way entirely. Probably take a more functional approach instead of iterating through a loop.

edit - The entity framework context has an ObjectDisposedException on the second pass.

People debate whether you should directly expose an EF context to consuming code. I say you shouldn't. Instead, mediate all interaction with the context in a class that understands how to manage the lifecycle of the context.

User0015
Nov 24, 2007

Please don't talk about your sexuality unless it serves the ~narrative~!

New Yorp New Yorp posted:

People debate whether you should directly expose an EF context to consuming code. I say you shouldn't. Instead, mediate all interaction with the context in a class that understands how to manage the lifecycle of the context.

That level of abstraction would be overkill for my dumb little site. This just reinforces my previous comment that I should have used EF for all the behind-the-scenes work (migrations, seeding, generating sql), but leave the actual grunt work of database transactions to something like Dapper.

On the other hand, I fixed my issue by taking a more functional approach. My current changes now attach an Action<> to an object along with all the necessary details for S3 to work, then awaiting/async on that, so now the image manages its own upload logic. Works 100% good job everyone.

User0015
Nov 24, 2007

Please don't talk about your sexuality unless it serves the ~narrative~!
Figured it out. Beware, friends, the magic of DI.

So having investigated further into EF and how it works, I stumbled on this blog post: https://thinkrethink.net/2017/12/22/cannot-access-a-disposed-object-in-asp-net-core-when-injecting-dbcontext/

At first I thought I was returning a Task void due to my async calls, but after ripping them all out, the dispose problem persisted. On a whim, I decided to check my DI container just in case. It's creating services as Scoped, so I dug into the EF core source code and discovered that the default setting for AddDbContext is also Scoped. I think, why not declare it Scoped directly. Whats the worse that could happen? Because by all rights this should have absolutely no effect whatsoever.

Answer: It is now working perfectly.

:suicide:

qsvui
Aug 23, 2003
some crazy thing
Is using a DI container considered a best practice in .NET? I've never really encountered the term "dependency injection" before I started using C# so I was just wondering.

EssOEss
Oct 23, 2006
128-bit approved
Yes, it is pretty much required for any nontrivial project.

Mr Shiny Pants
Nov 12, 2012

EssOEss posted:

Yes, it is pretty much required for any nontrivial project.

Why? Not a snarky question. :)

EssOEss
Oct 23, 2006
128-bit approved
As projects get large, manually wiring up dependencies becomes annoying and difficult. You want the lines of code you write to accomplish the business goal, not to carry variable A from class X to class Y to class Z just so it can be passed to method Q.

As manual dependency wiring gets hard, developers often add "shortcuts" like global variables to stuff values into and "creative" dependency graphs just to avoid passing around some pointless variables.

All of this makes for software that is difficult to modify and difficult to understand because of all the hardcoded dependency logic that needs to both be understood and that at the same time obscures the actual valuable part of the code. In short, your code becomes less maintainable over time.

With well-designed usage of injection containers, you can eliminate the vast majority of this code and, just as important, condense it in a single method per scope where all the dependency configuration is defined (instead of littering every file in your code with variable-passing lines).

EssOEss fucked around with this message at 10:35 on Jan 4, 2019

Sab669
Sep 24, 2009

I need to compare 2 sets of a handful of DLLs and figure out what the difference is. What's the best way to do that?

I have DotPeek installed, so I could go through each class in each DLL and manually copy-paste the content into Notepad++ and then WinMerge the text files but that sounds like a colossal pain in the rear end. Surely there's got to be a way to automagically do this?

If I simply use WinMerge itself on the DLLs I get not so helpful output:


I did find this blog but either we don't use (or I can't get today) NDepend / Reflector and that LibCheck Microsoft link 404's. JetBrains suggests this route but 'Export to Project' doesn't seem to be a thing for me? Is that a Premium feature or something?

Sab669 fucked around with this message at 13:58 on Jan 4, 2019

NihilCredo
Jun 6, 2011

iram omni possibili modo preme:
plus una illa te diffamabit, quam multæ virtutes commendabunt

Thing we learnt today from a legacy webservice at the cost of a very angry customer:

Even though the Windows registry is kept in RAM at all times and is normally super cheap to access, do NOT feel free to check a registry key on every HTTP request. Past a certain load it will start to bottleneck, and you may or may not waste several weeks trying to find that bottleneck by profiling the database calls / IO file access / webserver timeouts because the choke exclusively happens during prime time at noon on Saturdays when R&D is not on call, which was definitely a mixed blessing in this case.

B-Nasty
May 25, 2005

Another huge benefit of dependency injection, as used to support inversion of control (constructor-based, preferably), is that it allows you to mock out dependencies to easily unit test bit of your code. If, for instance, you had:

code:

public class FooProcessor{

   private IApiCaller apiCaller;

   public FooProcessor(IApiCaller apiCaller){
      this.apiCaller = apiCaller;
   }

   public int DoSomethingAndGetResult(){
      apiCaller.CallApi();
      //Other important logic here
   }

}
If you wanted to test the result of DoSomethingAndGetResult without actually calling the API, you could either write a fake implementation of IApiCaller and pass it in or use one the auto-mocking frameworks that do it for you.

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

B-Nasty posted:

Another huge benefit of dependency injection, as used to support inversion of control (constructor-based, preferably), is that it allows you to mock out dependencies to easily unit test bit of your code. If, for instance, you had:

code:

public class FooProcessor{

   private IApiCaller apiCaller;

   public FooProcessor(IApiCaller apiCaller){
      this.apiCaller = apiCaller;
   }

   public int DoSomethingAndGetResult(){
      apiCaller.CallApi();
      //Other important logic here
   }

}
If you wanted to test the result of DoSomethingAndGetResult without actually calling the API, you could either write a fake implementation of IApiCaller and pass it in or use one the auto-mocking frameworks that do it for you.

The discussion is more about "what's the benefit of DI containers" vs "what's the benefit of DI".

Personally, I dislike DI containers. They replace the boilerplate of do-it-yourself constructor injection/construction of dependency graphs with the boilerplate of telling the container how to construct dependencies, which in my experience just leads to not being able to figure out how the gently caress anything is constructed and makes troubleshooting failures resulting from misconfigured DI almost impossible.

B-Nasty
May 25, 2005

New Yorp New Yorp posted:

Personally, I dislike DI containers. They replace the boilerplate of do-it-yourself constructor injection/construction of dependency graphs with the boilerplate of telling the container how to construct dependencies, which in my experience just leads to not being able to figure out how the gently caress anything is constructed and makes troubleshooting failures resulting from misconfigured DI almost impossible.

But that's the whole point. I want an implementation of IFooProcessor, and I don't care if it's AzureFooProcessor or AwsFooProcessor as long as it implements the contract I'm using. That detail can be handled somewhere else, higher in the architecture.

What misconfiguration issues do you frequently experience? Note, I'm assuming no abusing of DI to do property injection / pseudo-config of variables and crap like that.

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

B-Nasty posted:

But that's the whole point. I want an implementation of IFooProcessor, and I don't care if it's AzureFooProcessor or AwsFooProcessor as long as it implements the contract I'm using. That detail can be handled somewhere else, higher in the architecture.

What misconfiguration issues do you frequently experience? Note, I'm assuming no abusing of DI to do property injection / pseudo-config of variables and crap like that.

This may just be my dinosaur-ism starting to show, but last time I was working on a ASP .NET Core project that used the built-in DI container, I was just constantly encountering dependencies not being resolved correctly and being totally at a loss as to how to troubleshoot it other than by looking through a huge list of declarations like

services.AddSingleton<IFooServiceConfiguration, FooServiceConfiguration>();
services.AddScoped<IFooService, FooService>();

and trying things.

I'm all for abstracting the construction of dependencies, I just typically encounter DI containers being great up until they aren't, at which point you invest more time in beating your head against the tool to get it to work properly than it would have taken you to just write some boilerplate to solve the same problem. See also: Macro-ORMs like EF/nHibernate vs Dapper. EF is great until writing a simple query makes it generate ridiculously bad SQL, at which point you're doomed to fight it for a few hours.

The equation is always "how many hours am I going to waste fighting the tooling vs how many hours is the tooling going to save me?". DI containers and heavy ORMs typically fall on the wrong side of that. Again, it may just be me being a dinosaur. :shrug:

New Yorp New Yorp fucked around with this message at 16:59 on Jan 4, 2019

User0015
Nov 24, 2007

Please don't talk about your sexuality unless it serves the ~narrative~!

New Yorp New Yorp posted:

The equation is always "how many hours am I going to waste fighting the tooling vs how many hours is the tooling going to save me?". DI containers and heavy ORMs typically fall on the wrong side of that. Again, it may just be me being a dinosaur. :shrug:

That's how I approach these things to. Simple is almost always better, and I'd take Dapper over EF/nHibernate in a heartbeat.

As far as declaration lists go, there's plenty of ways to create a resolver for DI so that it automatically wires up services that follow certain conventions. That way you don't end up with 100 lines of declaration. It's also maybe good since it enforces naming conventions, but it will often trip people up from time to time when they decide to try inventing their own.

Mr Shiny Pants
Nov 12, 2012
That is a bit what I was getting at, I am all for Dependency Injection, the problem starts when it wants to do more and more, making the whole thing a bit too obfuscated.

It becomes *magic* until, one day, it does not work anymore. And that day will come, we are working with computers after all.

EssOEss
Oct 23, 2006
128-bit approved
As with any tool, it is how you use it. It pains me to know that there is no "Using dependency injection containers 100, 200, 300" classes in university when people go to learn how to make software. We need to learn such things online, in blogs or by doing. And even when one does try to apply it, there is rarely someone nearby to guide in using it the right way. In our industry, the greatest dissapointment is the bad state of relevant education!

Ahem, anyway, I find that while people do occasionally gently caress up DI, it is far easier to later unfuck than manual dependency management (which also gets hosed up plenty).

brap
Aug 23, 2004

Grimey Drawer

EssOEss posted:

Yes, it is pretty much required for any nontrivial project.

I’m glad you had success with it, but you can’t generalize about this.

raminasi
Jan 25, 2005

a last drink with no ice
I will put in a vote for "DI containers save a shitload of work when they're working well but are terrible to debug when they're not, but on the balance I've preferred using them." I've only ever used Windsor, though - there might be enough variation in quality that the specific one you use matters. It is frustrating when something obscure breaks and you have to go bother the gurus though. I'd have a real rough time if I didn't have access to any of them.

Bruegels Fuckbooks
Sep 14, 2004

Now, listen - I know the two of you are very different from each other in a lot of ways, but you have to understand that as far as Grandpa's concerned, you're both pieces of shit! Yeah. I can prove it mathematically.

EssOEss posted:

As with any tool, it is how you use it. It pains me to know that there is no "Using dependency injection containers 100, 200, 300" classes in university when people go to learn how to make software. We need to learn such things online, in blogs or by doing. And even when one does try to apply it, there is rarely someone nearby to guide in using it the right way. In our industry, the greatest dissapointment is the bad state of relevant education!

Ahem, anyway, I find that while people do occasionally gently caress up DI, it is far easier to later unfuck than manual dependency management (which also gets hosed up plenty).

DI injection is not a challenging thing to set up - like honestly, just read this quick start for Simple Injector in .NET. Some other containers might be harder to work with but the basic principle is not difficult and it can be added to a project in a matter of minutes.

However, I've seen people get lazy with the registration configuration and started doing poo poo like combining auto-registration of classes with templates and doing confusing poo poo, and that definitely burned a few hours of my time.

EssOEss
Oct 23, 2006
128-bit approved
It's never the "set up", it is the "use responsibly over 5 years".'

brap posted:

I’m glad you had success with it, but you can’t generalize about this.

I just did, brah.

Bruegels Fuckbooks
Sep 14, 2004

Now, listen - I know the two of you are very different from each other in a lot of ways, but you have to understand that as far as Grandpa's concerned, you're both pieces of shit! Yeah. I can prove it mathematically.

EssOEss posted:

It's never the "set up", it is the "use responsibly over 5 years".'

Honest, I can't say that I haven't seen people gently caress up configuring a DI container or burn people's time using it, but the same people that will gently caress up DI container setup are the same people that gently caress up everything and shouldn't be coding.

Dietrich
Sep 11, 2001

There is never a reason not to use a DI container.

Windsor, StructureMap, AutoFac and Ninject all have good debugging tools that will tell you exactly what is wrong when it can't put something together for you, either in the exception the container throws or with a "AssertConfigurationIsValid()" type call that you're supposed to pop in after you finish registering everything.

RTFM dudes.

epswing
Nov 4, 2003

Soiled Meat
I use DI for pretty much anything non-trivial that I need to test and maintain in the long term. My thoughts are, how can you write real unit tests without inversion of control? Which leads to the next question, how are you going to build those IoC objects graphs without dependency injection? By hand? I guess you could, but why? I agree with Dietrich that any DI binding/registration errors I've encountered have been straightforward to debug/fix (so far :ohdear:).

epswing fucked around with this message at 03:55 on Jan 6, 2019

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

epalm posted:

I use DI for pretty much anything non-trivial that I need to test and maintain in the long term. My thoughts are, how can you write real unit tests without inversion of control? Which leads to the next question, how are you going to build those IoC objects graphs without dependency injection? By hand? I guess you could, but why? I agree with Dietrich that any DI binding/registration errors I've encountered have been straightforward.

Not using an IoC container doesn't mean you're not using IoC. I've always written it like this, when not using a container:

code:
public Foo(IBar bar) 
{
    this.bar = bar;
}
public Foo() : this(new RealBar()) 
{
}
When testing, I can construct an IBar through whatever mechanism I want (let's argue about whether mocking frameworks are worth it next! :)) and pass it in. In every other case, I get a RealBar. If RealBar has dependencies, they'll get constructed the same way, and RealBar can be tested by using the constructor overload.

[edit]
This is one of those topics I don't feel very strongly about in either direction. I haven't encountered a scenario where I saw an advantage to using an IoC container, but I don't deny that those scenarios exist. The few cases I've encountered them in Not Invented Here code have been unpleasant for me, but that certainly could be because they were implemented poorly. And my most recent experience was with the ASP .NET Core DI container, and given the relative immaturity of the .NET Core tooling, it wouldn't surprise me if the built-in DI was far behind the more mature third-party offerings in that space.

New Yorp New Yorp fucked around with this message at 04:09 on Jan 6, 2019

B-Nasty
May 25, 2005

New Yorp New Yorp posted:

code:
public Foo(IBar bar) 
{
    this.bar = bar;
}
public Foo() : this(new RealBar()) 
{
}

Ugh. So what do you do when, as is typical in the real world, Foo has 5 dependencies all of which have multiple dependencies themselves, ad infinitum? I prefer not to manage my dependency tree myself - we have containers that do that for us. I'd much rather change RealBar to BetaRealBar in one spot than have to dig all around the codebase to find where I new'd up a bunch of hard-coded dependencies.

kitten emergency
Jan 13, 2008

get meow this wack-ass crystal prison
i find DI to be a solution in search of a problem quite frequently. there's instances where it's useful (testability, etc.) but honestly a lot of it feels like people trying to be clever rather than clear. i'd rather debug non-clever code any day of the week.

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

B-Nasty posted:

Ugh. So what do you do when, as is typical in the real world, Foo has 5 dependencies all of which have multiple dependencies themselves, ad infinitum? I prefer not to manage my dependency tree myself - we have containers that do that for us. I'd much rather change RealBar to BetaRealBar in one spot than have to dig all around the codebase to find where I new'd up a bunch of hard-coded dependencies.

code:
public RealBar(IBaz baz) { ... }
public RealBar() : this(new RealBaz()) {}

public RealBaz(IQux qux, IQaz qaz) {...}
public RealBaz() : this(new RealQux(), new RealQaz()) {}

public RealQux(IThinkYouGetTheIdea ) ....
The dependency chain is self-constructing. There's an assumption here (which, to be fair, isn't always the case) that there's a single "real" implementation and any other implementations are for testing purposes, as opposed to something that is truly using an interface to allow for multiple implementations.

epswing
Nov 4, 2003

Soiled Meat

New Yorp New Yorp posted:

Not using an IoC container doesn't mean you're not using IoC. I've always written it like this, when not using a container:

Right. I agree.

I suppose what I should have said was "I use IoC for pretty much anything non-trivial that I need to test and maintain in the long term, and I tend to use DI as a tool to facilitate this."

sadus
Apr 5, 2004

+1 for Autofac, modules are really handy in giant projects with multiple appdomains, and being able to control the scope of registrations is invaluable.

B-Nasty
May 25, 2005

I would add that another cool thing containers can enable is supporting a fully-disconnected mediator pattern of messages and message handlers e.g. https://github.com/jbogard/MediatR/wiki

I mean, I suppose you could wire it all up manually in the resolver, but that takes so much of the fun out of it.

epswing
Nov 4, 2003

Soiled Meat
I have some WebAPI error handling questions.

C# code:
public class MyController : ApiController
{
    [HttpPost]
    public void Foo()
    {
        throw new Exception();
    }
    
    [HttpPost]
    public void Bar()
    {
        try
        {
            throw new Exception();
        }
        catch (Exception ex)
        {
            log.Error(ex);
            throw ResponseException(Request, ex);
        }
    }
    
    public static HttpResponseException ResponseException(HttpRequestMessage request, Exception e) =>
        new HttpResponseException(
            request.CreateErrorResponse(HttpStatusCode.BadRequest, new HttpError(e, true))
        );
}
When I call Foo, the response HTTP status is 500 InternalServerError, and the response body is
JSON code:
{
    "Message": "An error has occurred.",
    "ExceptionMessage": "oops",
    "ExceptionType": "System.Exception",
    "StackTrace": "   at Blah.MyController.Foo() in C:\\Project\\Controllers\\Api\\MyController.cs:line 37
       at lambda_method(Closure , Object , Object[] )
       at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ActionExecutor.<>c__DisplayClassc.<GetExecutor>b__6(Object instance, Object[] methodParameters)
       at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ActionExecutor.Execute(Object instance, Object[] arguments)
       at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ExecuteAsync(HttpControllerContext controllerContext, IDictionary`2 arguments, CancellationToken cancellationToken)
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Web.Http.Controllers.ApiControllerActionInvoker.<InvokeActionAsyncCore>d__0.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Web.Http.Controllers.ActionFilterResult.<ExecuteAsync>d__2.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Web.Http.Dispatcher.HttpControllerDispatcher.<SendAsync>d__1.MoveNext()"
}
When I call Bar, the response HTTP status is 400 BadRequest, and the response body is
JSON code:
{
    "Message": "An error has occurred.",
    "ExceptionMessage": "oops",
    "ExceptionType": "System.Exception",
    "StackTrace": "   at Blah.MyController.Bar() in C:\\Project\\Controllers\\Api\\MyController.cs:line 39"
}
Aside from Foo's stack trace showing more detail, the response body is the same.

Firstly, the status code. Sometimes, the user did something wrong and returning 400 is correct. Sometimes, I have done something wrong, and returning 500 is correct. What strategies do you use to return the right status code? I could wrap all user-generated errors in some top-level AppException, catch AppException in every controller action, and generate an appropriate 400 as Bar does above. Otherwise, the exception will skip that catch, and become a 500. Do people do this?

Secondly, I want to log exceptions. Is there some way other than wrapping every controller action in the same boilerplate try/catch?

Third question, is there a circumstance where the client won't receive a json object with Message, ExceptionMessage, ExceptionType, and StackTrace fields? Or can I count on this structure always being the same when an ApiController throws an exception?

epswing fucked around with this message at 05:00 on Jan 6, 2019

Mr Shiny Pants
Nov 12, 2012
What a user does should not cause an exception on your server and the right handling should be in place.
I would not use exceptions for this, crappy input is not exceptional. :)

At least, that is how I try to write my code.

ThePeavstenator
Dec 18, 2012

:burger::burger::burger::burger::burger:

Establish the Buns

:burger::burger::burger::burger::burger:

epalm posted:

Firstly, the status code. Sometimes, the user did something wrong and returning 400 is correct. Sometimes, I have done something wrong, and returning 500 is correct. What strategies do you use to return the right status code? I could wrap all user-generated errors in some top-level AppException, catch AppException in every controller action, and generate an appropriate 400 as Bar does above. Otherwise, the exception will skip that catch, and become a 500. Do people do this?

You don't need to throw an exception if you're returning a response right from the controller. The way I usually break down responsibility of each layers in your typical web service is with the controller having 2 responsibilities: validate incoming request parameters/headers/whatever, and then respond with an appropriate response code from the result it receives from the service layer. If you're validating input in the controller, you can just return BadRequest() without throwing an exception if the inputs are not valid. You shouldn't be using exceptions for controlling execution flow in general.

epalm posted:

Secondly, I want to log exceptions. Is there some way other than wrapping every controller action in the same boilerplate try/catch?

Use Application Insights if you're using .NET and especially if you're using .NET and hosting on Azure. Gives you a ton of metrics right out of the box with no configuration and is very customizable.

epalm posted:

Third question, is there a circumstance where the client won't receive a json object with Message, ExceptionMessage, ExceptionType, and StackTrace fields? Or can I count on this structure always being the same when an ApiController throws an exception?

You shouldn't be exposing these through your web API. It can be a security list because that stack trace could potentially include things like query strings or worse, configuration information. Even if you're careful to only throw certain exceptions and it's internal use only, you've now coupled the consumers of your API to C# stack traces that are thrown by .NET Core default HTTP error responses. Ideally you're writing web APIs so that if you wanted to, you could rewrite them in a completely different framework/language and not break compatibility with existing consumers. Older ASP.NET projects don't even throw the same errors, they return full error HTML pages or XML in the response.

What you should do is return a standard message with just enough information to tell the consumers what the problem is. It could be as simple as a static string, or as verbose as a JSON blob with a bunch of descriptive status fields.

e:

Mr Shiny Pants posted:

What a user does should not cause an exception on your server and the right handling should be in place.
I would not use exceptions for this, crappy input is not exceptional. :)

At least, that is how I try to write my code.

Yeah this is pretty much it. As far as exceptions go, here are the codes you're going to end up returning most often:

400 Bad Request - No exception thrown, you validate the incoming request and respond with this if it's not valid
401 Unauthorized - Usually is going to be baked in to your web app framework
403 Forbidden - You'll have to write the code for this usually, request is well-formed and the user is authorized to use the service but lacks the permissions for that request. No exception needed.
404 Not Found - Didn't find what was requested. Usually it'll be because your data access layer returned null/empty/empty list when you queried your database or another web service. No exception needed.
405 Method Not Allowed - Usually is going to be baked in to your web app framework
500 Internal Server Error - Some exceptional condition that isn't being handled happened while trying to process the request. Exception thrown, but you should still probably not expose a full stack trace to the consumer.

ThePeavstenator fucked around with this message at 06:04 on Jan 6, 2019

downout
Jul 6, 2009

Anyone have some good recommended reading regarding DI? I've used it in various settings, but it was always more from a format of "this is how we do it, what we use, just keep doing it that way". It was never covered in my formal education (and mostly abstracted away by libraries in my post-education experience), so I think it might be really helpful to see a more formal usage/construction in real world scenarios.

downout fucked around with this message at 06:25 on Jan 6, 2019

Mr Shiny Pants
Nov 12, 2012

downout posted:

Anyone have some good recommended reading regarding DI? I've used it in various settings, but it was always more from a format of "this is how we do it, what we use, just keep doing it that way". It was never covered in my formal education (and mostly abstracted away by libraries in my post-education experience), so I think it might be really helpful to see a more formal usage/construction in real world scenarios.

DI? or DI containers?

Adbot
ADBOT LOVES YOU

downout
Jul 6, 2009

Mr Shiny Pants posted:

DI? or DI containers?
DI, but I wouldn't turn down any good suggestions on both. DI primarily tho.

  • 1
  • 2
  • 3
  • 4
  • 5
  • Post
  • Reply