Aruncare erori intentionate prea multe?

Deoarece incerc sa pastrez controllerul cat mai curat, ce contine un singur request catre serviciu si un log, toate validarile le-am mutat in serviciu, iar daca vre’una nu este indeplinita, atunci arunc o ApplicationException cu un ErrorHandler convertit in string.

Cum arata o metoda din Controller:

        [HttpPost]
        public async Task<IActionResult> Create([FromBody] DataSource model)
        {
            Data.Entities.User user = null;
            string className = nameof(DataSourcesController);
            string actionName = nameof(DataSourcesController.Create);
            string ip = string.Empty;
            string msg = string.Empty;

            try
            {
                var requestData = await GetRequestData();
                user = requestData.CurrentUser;
                ip = requestData.IpAddress;

                var createdModel = await dataSourcesService.Create(model, user.Id);
                msg = "Created new data source";
                await loggerService.LogInfo(className, actionName, msg, user, ip);

                return Ok(createdModel);
            }
            catch (Exception ex)
            {
                return await HandleError(ex, className, actionName, user, ip);
            }
        }

        private async Task<IActionResult> HandleError(Exception ex, string className, string actionName, Data.Entities.User user, string ip)
        {
            try
            {
                if (ex.GetType() == typeof(ApplicationException))
                {
                    var err = JsonSerializer.Deserialize<ErrorType>(ex.Message);
                    var clientMsg = !string.IsNullOrEmpty(err.ClientMessage) ? err.ClientMessage : err.ServerMessage;

                    await loggerService.LogWarning(className, actionName, err.ServerMessage, user, ip);
                    if (err.StatusCode == 400)
                    {
                        return BadRequest(clientMsg);
                    } 
                    else if (err.StatusCode == 409)
                    {
                        return StatusCode(409, clientMsg);
                    }
                }
                else
                {
                    await loggerService.LogError(className, actionName, ex.Message, user, ip);
                }
            }
            catch { }

            return StatusCode(500);
        }

Serviciul:

    public async Task<DataSource> Create(DataSource model, int userId)
    {
        await packagesService.CanCreateDataSource(userId);

        await CheckNameAvailable(model.Id, model.Name, userId);

        var newEntity = new Data.Entities.DataSource()
        {
            Name = model.Name,
        };

        dataContext.Add(newEntity);
        await dataContext.SaveChangesAsync();

        return await GetById(newEntity.Id);
    }
        public async Task<bool> CanCreateDataSource(int userId)
        {
            var status = await GetStatus(userId);
            if (status.CurrentDataSources < status.MaxDataSources)
            {
                return true;
            }

            var error = new ErrorType()
            {
                StatusCode = 403,
                ClientMessage = $"You have reached the limit of allowed data sources for your package.",
                ServerMessage = $"User #{userId} reached limit of allowed data sources."
            };

            throw new ApplicationException(JsonSerializer.Serialize(error));
        }

De exemplu, inainte sa creez entitatea, verific prin metoda CanCreateDataSource daca operatiunea este valida. Metoda respectiva returneaza “true” (ceea ce ar trebui sa fie un void de fapt, pentru ca nu fac nimic cu rezultatul) sau arunca o exceptie ca mai sus. O alta problema urata este ca obiectul pe care il trimit ca exceptie trebuie convertit in string (ApplicationException accepta doar string), iar in handlerul controllerului, verific tipul exceptiei, dupa care convertesc stringul in obiect. Am facut asta pentru ca am nevoie ca in db sa salvez un mesaj al erorii, iar in client sa trimit un alt mesaj, “personalizat”.

Am destul de multe validari care arunca exceptii tocmai pentru ca le-am mutat in servicii, si nu puteam returna direct de acolo tipuri de HttpResponse. Mi s-a parut cea mai buna solutie gasita pana acum. Ce parere aveti? :smiley: Ma gandesc ca la un numar mai mare de requesturi sa nu fie probleme cand serverul va ajunge sa primeasca zeci de ApplicationExceptions pe zi

Eu inteleg ca await / async e pt multithreading, dar nu inteleg, cum te ajuta, daca nu faci mai multe apel-uri in parallel si apoi astepti ca toate sa se terminte, nu vad cum te ajuta await, nu e nici o differeta intr-o o functie normal si una cu await, cate una asa.

Scuze de offtopic dar ma contrariat asta in NET, peste tot await, dar niciodata nu foloseam functiile alea in parallel, doar secvential.

Async/await nu e neaparat pentru multi-threading ci pentru operatii asincrone care ar putea fi pe un alt thread sau ar putea fi un call catre un server.

@AlleXyS Poate ar trebui sa iti faci niste clase ce deriveaza din Exception care sa le prinzi in catch.

Personal nu mi se pare ok ca metoda CanCreateDataSource arunca exceptie in conditiile in care te-ai astepta de fapt sa iti returneze fals (eventual doar daca primeste argumente invalide).

Daca e vorba de zeci pe zi nu e problema, poate la zeci pe secunda o sa fie altceva.

de ce m-ar interesa sa fie pe un alt thread? ce are thread-ul current, oricum e blocat in asteptare, de ce nu ar face thread-ul current “munca”… propriu-zis.

Relax, nu ai probleme nici la sute de exceptii per secunda.

Daca este pe o aplicatie de desktop ii destul de naspa sa se blocheze gui-ul in timp ce face in fundal operatii pe retea sau io
In spate compilatorul genereaza un state machine

1 Like

Ideea e sa nu te intereseaza unde e, ca e in mod direct pe un alt thread din programul tau, ca e pe un server sau ca pe un alt thread in mod indirect pentru ca e un alt program (gen baza de date). Dar thread-ul curent nu se blocheaza, in momentul in care faci await se returneaza un obiect the tip Task catre caller iar codul care e dupa await se va executa dupa ce operatiile din Task se termina asemanator cu atunci cand le-ai fi pus intr-un callback (e mai complex de atat, stiu ca runtime-ul face in spate un state machine si ca intervine si un task scheduler pe undeva dar nu am intrat asa de adanc in detaliile de implementare).

Da, asta e una dintre chestiile pentru care e folosit dar in exemplu de fata nu e vorba de GUI ci de un API. In cazul asta e important sa nu se blocheze threadul pentru a putea procesa alte request-uri intre timp.

2 Likes

Stiu :slight_smile:

Eu am dat un exemplu cu un efect mai vizibil.

1 Like

Inteleg, Multumesc, deci, nu e o blocare tehnica, dar e o blocare practica, executia asteapta dar nu blochezi thread-ul current, destul de smart.

multumesc pentru raspunsuri.

@konn Cred ca din obisnuinta (sau lipsa de cunostinte prea avansate) folosesc async/await acolo unde folosesc Entity Framework (practic cam pentru toate interogarile pe baza de date, unde EF are functii async).

Avantajul pe care m-am gandit eu ca-l voi avea daca arunc ApplicationException acolo unde conditia nu este indeplinita ar fi ca elimin verificarile inutile (if-urile) intrerupand executia taskului,

exemplu cu throw ApplicationException:

public async Task<User> CreateUser(user) {
     await canCreateUser(); // void if ok, throw application exception if not
     await checkNameAvailablility();  // void if ok, throw application exception if not
     
    context.Add(user);
    await context.SaveChangesAsync();
    return await GetById(user.Id);
}

Exemplu cu if !! Pe langa asta, ar trebui sa returnez un tip generic:

public async Task<GenericResponse<T>> CreateUser(user) {
   var canCreate = await canCreateUser(); // true if ok, false if not
   if (!canCreate) {
     return new GenericResponse<string>() {
         StatusCode = 403,
         Data = "Can not create user"
      };
    }

   var nameAvailaible = await checkNameAvailablility();
   if (!nameAvailaible ) {
     return new GenericResponse<string>() {
         StatusCode = 403,
         Data = "Name already taken"
     };
    }

    context.Add(user);
    await context.SaveChangesAsync();

     return new GenericResponse<User>() {
         StatusCode = 200,
         Data = user
      };
}

@Cosmin_Popescu Consumatorul api-ului este un SPA, deci nu m-am gandit ca o sa influenteze/blocheze cu ceva deoarece din eroare intorc in client un mesaj de forma string.

Am vazut aplicatii mari care aplica formatul pe care l-am descris mai sus, nu returneaza BadRequest-uri in cazul verificarilor, ci Ok-uri ce contin obiecte cu campurile StatusCode si Data (un fel de client BadRequest integrat in OK-ul serverului). Adica un Ok cu StatusCode 400 ce lasa clientul sa decida ce face cu raspunsul.

Folosind exemplul de mai sus, controllerul ar returna intotdeauna Ok(GenericResponse), iar exceptiile reale vor fi prinse in catch-ul blocului. // Daca ma gandesc mai bine, dupa ce am scris exemplul asta, pare mai bun decat metoda pe care am ales-o pana acu :laughing: las erorile reale pentru catch, iar actiunile invalide sunt returnate ca Ok cu statusCodeuri personalizate.

@TGeorge, ApplicationException insasi deriveaza din Exception si de-asta am folosit-o pana acum, prinzand-o in catch-ul controllerului, inainte de alte exceptii. (ori am inteles eu total gresit?)

Am incercat sa raspund tuturor, merci :crazy_face:

1 Like

De regula, sunt 2 abordari posibile cand vine vorba de Web APIs:

  1. Returnezi mereu OK (200), iar mai departe clientul este responsabil sa interpreteze daca raspunsul este cu succes sau cu eroare, in functie de o anumita conventie folosita atunci cand construiesti raspunsul (de exemplu poti sa ai o proprietate success: true/false pe toate raspunsurile de API).

  2. Returnezi coduri de eroare Http (400, 401, 500, etc.), iar clientul trateaza aceste erori.

Ambele variante au pro si cons, important este sa decideti in echipa/proiect varianta pe care vreti s-o folositi si sa va tineti de ea peste tot.

Eu fac si Web APIs si frontend si personal prefer sa returnez coduri specifice Http si cat mai putine raspunsuri generice. De exemplu, nu imi place sa returnez din API: Task<GenericResponse<T>> sau Task<IActionResult>. Prefer sa returnez Task<User> sau Task<UserCreated> sau Task<bool>. De ce? Pentru ca vreau sa ma aliniez cu standardul OpenAPI si poate chiar sa generez clientul automat cu swagger-gen, de exemplu.

@AlleXyS
Referitor la error handling, ar trebui sa faci un global error handler printr-un FilterAttribute (.NET Web API) custom. Astfel nu mai trebuie sa pui niciun try/catch in niciun controller.

Uite cum arata un controller de-al meu, nu contine niciun fel de logica:

    [ApiVersion("1")]
    [RoutePrefix("api/v{version:apiVersion}/details")]
    public class DetailsController : ApiController
    {
        private readonly IMediator _mediator;

        public DetailsController(IMediator mediator)
        {
            _mediator = mediator;
        }

        [HttpGet, Route("{objectId}/{recordId:int}/layout")]
        public async Task<DetailsLayout> GetDetailsLayout(string objectId, int recordId)
        {
            return await _mediator.Send(new GetDetailsLayoutQuery(objectId, recordId));
        }

        [HttpGet, Route("{objectId}/{recordId:int}/content")]
        public async Task<DetailsContent> GetDetailsContent(string objectId, int recordId)
        {
            return await _mediator.Send(new GetDetailsContentQuery(objectId, recordId));
        }

        [HttpGet, Route("{objectId}/{recordId:int}/content/{pageId:int}")]
        public async Task<DetailsContent> GetDetailsPageContent(string objectId, int recordId, int pageId)
        {
            return await _mediator.Send(new GetDetailsPageContentQuery(objectId, recordId, pageId));
        }
    }

Faptul ca nu vezi niciun try/catch in controller nu inseamna ca nu exista error handling sau logging. Exista chiar si request validation (cu FluentValidation), doar ca toate acestea se intampla printr-un mecanism de tip pipeline.

4 Likes

In principiu, async/await ajuta cand aplicatia ta face multe operatii I/O. Citirea si scrierea intr-o baza de date sunt operatii I/O si de cele mai multe ori asta adauga o oarecare latenta procesarii request-urilor dintr-un server web. La fel si orice call catre un serviciu extern sistemului tau ar trebui facut printr-un mecanism async/await.

In schimb, daca ai o aplicatie care face foarte multe calcule, deci CPU-intensive, atunci nu se mai recomanda async/await, dimpotriva. In aceasta situatie async/await poate afecta performanta sistemului.

Async/await a aparut pentru ca resursele unui server sunt limitate fizic de numarul de procesoare sau CPU cores. Asta inseamna ca un server poate procesa un numar limitat de request-uri cu adevarat in paralel, chiar daca in realitate “pare” ca poate procesa milioane de request-uri in paralel. De fapt le proceseaza doar foarte repede.

Orice request care intra intr-un web server ruleaza pe un thread (sau worker) separat. Daca ai un web server cu 64 CPU cores, atunci acesta poate procesa fix 64 de request-uri cu adevarat in paralel, sau 128 daca are si hyper-threading.

Acum, daca ai o aplicatie web care foloseste o baza de date care, de cele mai multe ori, sta pe o alta masina, web-serverul tau va tine blocat acel thread pana cand proceseaza baza de date query-ul respectiv.

Async/await a aparut cand niste oameni destepti s-au gandit sa “elibereze” acele threaduri si sa permita serverului web sa foloseasca resursele pentru alte “incoming requests” intretimp.
Cand baza de date (sau orice alta operatie I/O sau serviciu extern) termina de procesat, se face “resume” la request-ul initial, dar de fapt se creeaza un nou thread, se face “context sync” si se continua procesarea request-ului.

Operatia de “context-sync” are un cost, de asta in anumite situatii async/await poate afecta performanta sistemului. De exemplu, daca baza de date ruleaza pe aceeasi masina fizica ca si aplicatia web, atunci nu se recomanda async/await.

9 Likes

Cum abordezi cazurile in care intr-o anumita metoda din controller vrei sa ai error handling specific ptr o anumita exceptie ?

Ce inseamna handling specific pentru o anumita exceptie? Pentru mine exceptiile sunt de cateva tipuri si de regula sunt prinse si tratate in cascada:

  • authentication/authorization exception
  • validation exception sau bad request, cand inputurile sunt incomplete
  • business exception - tot un fel de validation exception dar server side mai mult; de exemplu o denumire care nu trebuie sa fie duplicat in baza de date, etc.
  • connection exception - in cazul in care web api-ul meu face niste call-uri catre alte servicii externe, iar acestea nu raspund in timp util
  • unhandled exception - toate celelalte - server error sau http status code 500, cu un mesaj corespunzator sau chiar cu mesajul din exceptie, intrucat genul asta de exceptie ar trebui sa fie foarte rar.

Din experienta mea, nu ai nevoie de foarte multe tipuri de exceptii, iar un BusinessException poate cuprinde o arie larga de exceptii cand tot ce te intereseaza este un mesaj prietenos din care utilizatorul sa inteleaga rapid care este problema.

La business exception ma refeream. Mersi de info.

Nu-mi place prima abordare (de care mi-am dat seama dupa ce am deschis threadul asta) pentru ca muta verificarile in client. Adica, daca stii ca primesti response 200 cu obiect ce contine {statusCode si data}, va trebui sa faci mereu verificari pentru acel statusCode: 200, 201, 204, 400ocazional, 403, 409, cam astea ar fi valide (ce-mi vin in minte, bine, pana la urma tot un handler ar fi ca si in cazul erorilor reale), in rest 400, 401 si 404 raman erori standard. Deci cred ca raman la varianta de dinainte, cel putin pentru moment :grin:

De cand incerc sa reduc liniile din Controller, am reusit sa duc in servicii logica in mare parte, dar mai am cateva loguri si HttpHeaders (cred ca asa se numesc, de unde iau ip-ul clientului), n-am prea avut timp sa incerc sa le duc si pe astea mai in spate. La fel si cu try catch’ul (aici trebuie si putina documentatie)

In sfarsit am aflat si eu ce fac acele CPU cores :sweat_smile: Folosesc EF Core si peste tot am vazut ca se recomanda folosirea functiile async. (Baza de date e pe aceeasi masina cu api-ul :crazy_face:)

foarte clara categorisirea asta :dizzy_face:

multzam