Dilemă tratare erori în PHP

Limbaje de Programare PHP
1

Să stabilesc contextul, deşi nu e foarte relevant: fac o aplicaţie care va permite utilizatorilor să platească cu cardul încarcarea abonamentelor Focus Sat.

Dilema mea este una pur filosofică, legată de modul în care pot fi tratate erorile care pot apărea în momentul în care îmi vine notificare de la euplatesc.ro, iar aplicaţia mea va trebui să încerce să încarce abonamentul.

Callback-ul apelat de euplatesc.ro apelează la rândul lui API-ul de la Focus Sat, iar aici erorile pot fi diverse (autentificare eşuată, timeout, smartcard incorect etc).

Well, în acest punct este foarte tentant să adaug în log o intrare în care să descriu detaliile erorii şi execut die(“ERR”) exact în punctul în care mi-a apărut eroarea, un soi de “sudden death”. Abordarea asta este tentantă pentru cel care dezvoltă aplicaţia, pentru îi simplifica mult munca. Însă va fi un coşmar pentru nefericitul care va trebui să întreţină în viitor aplicaţia, pentru că aplicaţia va avea exit-uri random peste tot.

Exemplu de cod:

    [...]
    auth_focus($username, $password);
    [...]

    function auth_focus($username, $password)
    {
        $result = executa_api_focus();
        if(!$result)
            die("ERR");
    }

Cealaltă abordare este să laşi caller-ul să se ocupe să decidă ce să facă cu eroarea, callee-ul urmând doar să-l anunţe că s-a întamplat ceva. Codul va arăta mai profi, dar s-ar putea da în extrema cealaltă, pe care o detest la fel de mult, şi anume over-engineering-ul. Exemplu de cod.

    [...]
    if(auth_focus($username, $password))
        die("ERR");
    [...]

    function auth_focus($username, $password)
    {
        $result = executa_api_focus();
        if(!$result)
            return false;
    }

O altă abordare ar fi una intermediară, bazată pe excepţii. Exit-urile tot ar fi random, dar măcar ar fi în limitele blocului try/catch, deci nu ar fi la fel de mult de săpat ca în varianta sudden-death.

Exemplu cod:

    [...]
    try
    {
        auth_focus($username, $password);
        incarca_abonament($serie_smartcard, $tip_abonament);
    }
    catch(Exception $e)
    {
        error_log($e->getMessage());
        die("ERR");
    }
    [...]

    function auth_focus($username, $password)
    {
        $result = executa_api_focus_auth();
        if(!$result)
            throw new Exception("Parola incorecta");
    }

    function incarca_abonament($serie_smartcard, $tip_abonament)
    {
        $result = executa_api_focus_incarca_abonament();
        if(!$result)
            throw new Exception("Serie smartcard incorecta");
    }

M-ar interesa cum procedaţi voi, eventual cu nişte argumente. Sau poate există variante care la care nu m-am gândit.

2

din experienta mea (nu stiu cat de relevanta)… am ajuns la concluzia ca nu toate erorile sunt nascute egal.
in functie de fluxul de lucru… ai anumite puncte critice (cum e cel descris de tine) in care vrei sa tratezi eroarea cat mai bine dpdv tehnic pentru ca are impact asupra unei parti importante a aplicatiei (legate de plata).

mie mi se pare varianta a 2a de urmat aici pentru ca s-ar putea ca in loc de “die” sa vrei sa declansezi si un triggrer care sa mute executia pe un flux dedicat.

una e sa ii spui utilizatorului care vrea sa vada cand ii expira abonamentul ca a aparut o eroare de autentificare si nu ii poti da raspunsul si alta e cand omul a platit si se asteapta sa primeasca produsul.

oricum, astea ar trebui sa vina din partea de business a aplicatiei, tehnicul doar se conformeaza.
eu cred ca din 100 de erori posibile intr-o aplicatie… or sa fie 5 catre trebuie tratate extra-atent si 95 de care nu ii va pasa nimanui nicicand.

3

Sigur că erorile nu au toate aceeaşi importanţă (iar unele le tratez doar pentru că este posibil să apară, în realitate este foarte puţin probabil să se întâmple). Dar în general este o idee bună să aplici regulile în mod uniform, cel care va întreţine codul tău peste 5 ani să nu stea să ghicească cum ai făcut acolo şi cum ai făcut dincolo.

4

de acord, cu conditia sa iti permiti.
raman la parerea ca pentru exemplul pucntual e necesara tratarea “cu varianta 2”, pentru restul aplicatiei ramanand sa decizi daca ai resurse sa o faci uniform / consistent, sau aplici o metoda hibrida si folosesti “varianta 1” acolo unde nu conteaza.

5

as merge pe varianta cu aruncatul exceptiilor. ai avea o zona mai curata unde sa tratezi erorile + ti-ar permite sa iei diferite decizii in functie de exceptia prinsa. bonus, ar fi mult mai lizibil

6

Până la urmă metoda asta am ales-o, nu e perfectă, dar cu proiectare atentă poate să ajute şi developerul şi maintaner-ul.

7

Cateva observatii din experienta mea:

Indiferent de metoda de tratare a erorilor folosita, codul va arata ciudat. Ca e vorba de return 0, false, null. Ca e vorba de a salva mesaje, ca e vorba de a genera exceptii, codul va fi 50% functionalitate, 50% tratarea erorilor;

Nu folositi die(); niciodata.

Rolul unei aplicatii e sa functioneze/ruleze/sa se execute cap-coada. Niciodata un proces nu trebuie finalizat brusc (folosind acest die(); ). De fiecare data cand vin date de undeva ele trebuie validate si daca nu sunt valide procesul trebuie sa continue cu o multime vida ca input. Orice proces/functie trebuie sa lucreze cu 0 date de intrare. Te astepti sa primesti un array de tari, ei bine, functia aia trebuie sa mearga daca primeste un array cu 0 tari. Sau daca primesti un string sau un int (daca limbajul nu obliga utilizarea unui tip de date fix).

8

În general folosesc die() acolo unde suspectez că pot apărea atacuri bruteforce. De exemplu în acel callback de la euplatesc.ro mă aştept să primesc întotdeauna un răspuns corect. Dacă lipseşte un câmp, dacă e în plus un câmp, dacă suma de control este incorectă, îi tai instant macaroana, nu mai stau la discuţii să irosesc resurse cu alte check-uri.

9

In acest context as construi un mecanism de capturare a adreselor care genereaza aceste cereri frauduloase. Apoi aceasta lista as livra-o web server-ului ca el sa blocheze instant accesul de pe aceste IP-uri indiferent ce resursa ar cere.

1 Like
10

După ce dăm drumul la serviciu o să urmărim dacă apar cereri invalide şi dacă e cazul o să le trimit direct spre firewall, nici măcar să nu mai ajunga la webserver. Dar nu mă aştept să fie cine ştie ce, pentru că link-ul de callback nu este afişat nicăieri, dacă il nimereşte vreun bot o să fie pură întâmplare.

11

Suna bine.

In general depinde de buget. Solutii sunt de la un simplu die(); la ditamai infrastructura care comunica cu Interpol-ul.

Dar fundamentul ramane, procesul trebuie sa se execute cap-coada. Exista exit(); sau return; in loc de die();

12

Dacă nu mă înşel, die() este echivalent cu exit().

https://www.php.net/manual/en/function.exit.php

13

Use exit(); :slight_smile:

14

https://www.php.net/manual/ro/function.die.php

It is poor design to rely on die() for error handling in a web site because it results in an ugly experience for site users: a broken page and - if they’re lucky - an error message that is no help to them at all. As far as they are concerned, when the page breaks, the whole site might as well be broken.

1 Like
15

Well, nu întotdeauna servim pagini html. Uneori poate să fie un apel AJAX, care aşteapă un text chior sau un json sau cine ştie ce altă chestie. Sau poate să fie o comunicare server-to-server, cum este cazul callback-urilor din sistemele de e-payment. Cum am zis, die/exit este util când suspectezi că clientul încearcă să facă request-uri invalide, să-i tai rapid macaroana.

În rest, sunt de acord, pentru mine die/exit este echivalent cu goto, evit construcţiile care distrug flow-ul natural al execuţiei.

De fapt, dacă am fi mai catolici decât papa, ar trebui să evităm construcţiile de genul:

if(!$conditie1)
    return;

if(!$condtie2)
    return;

si trebui s-o facem asa:

if($conditie1)
{
    if($conditie2)
    {
        // executa cod
    }
}
16

si raspunsul ala primit trebuie sa fie uniform. nu vrei sa ai 10 de cazuri unde tratezi raspunsuri null, invalide sau obilecte cu diferite proprietati.

17

Prima varianta este 100% modul in care trebuie sa scrii cod. Se numeste early return, poti sa cauti mai multe informatii despre asta. Cu cat codul este mai indentat cu atat pierzi, ca programator sirul ideilor, dar asta e alta discutie de clean code.

Eu merg intotdeauna pe varianta cu aruncat de exceptie dar nu o exceptie generala cum ai prezentat ci de exceptie particulara care sa extinda \Exception, si asa poti face handle la exceptii in mod diferit in functie de necesitati. Daca vrei te poti folosi si de un anumit cod de exceptie ales de tine ca sa poti testa exceptiile alea, daca e nevoie.

E cel mai curat mod cu exceptiile aruncate si tratate unde este punctul critic, asa mai poti face retry in catch poate e down serviciul si a doua oara merge, poti face revert la tranzactie daca cade o parte din chain-ul necesar tie s.a.m.d.

2 Likes
18

E de dezbătut şi utilizarea early return-ului. În general îl folosesc fără remuşcări, pentru că ştiu că după el datele de intrare sunt valide 100%. Dar, la fel ca in cazul die(), e chestia cu exit-urile random din funcţie, care ar putea da dureri de cap celui care asigură mentenanţa.

Un punct de vedere interesant, sunt absolut de acord cu prima propoziţie:

In general, you want to have as few return points in a function as possible. The practical reason for this is that it simplifies your reading of the code since you can just always assume that each and every function will take its arguments, do its logic, and return its result.
Putting in extra returns for various cases tends to complicate the logic and increases the amount of time necessary to read and fully grok the code.
Once your code reaches the maintenance stage then multiple returns can have a huge impact on the productivity of new programmers as they try to decipher the logic (it’s especially bad when comments are sparse and the code unclear).
The problem grows exponentially with respect to the length of the function.

Aş avea grijă şi la extinderea clasei Exception, prea multe clase mai mult ar strica decât ar ajuta. Over engineering-ul poate fi la fel de dăunător ca improvizaţia.

19

De obicei, cu o structura, lucrurile arata destul de curat in PHP cu early returns:

public function setPerson($fname, $lname, $age)
{
  if (empty($fname)) {
    $this->error = "Missing first name";

    return false;
  }

  if (empty($lname)) {
    $this->error = "Missing last name";

    return false;
  }

  if (empty($age) || !is_numeric($age) || $age < 0) {
    $this->error = "Invalid age given";

    return false;
  }

  $this->greeting = "Hello {$fname} of the clan {$lname}. Great to have a {$age} year old among us!";

  return true;
}

In felul asta te astepti intotdeauna ca la inceputul unei metode sa ai tratate lucrurile ce pot cauza probleme si apoi ai corpul propriu-zis.

Desigur, intr-un mod mai profesional avem o clasa de validare:

public function setPerson($fname, $lname, $age)
{
   $validator = new PersonValidator($fname, $lname, $age);
   
   if (!$validator->passed()) {
     $this->error = $validator->getFailedMessages();  // Can be an array

     return false;
   }
   
   // ...

  return true;
}
20

Eu în cazul expus de tine am o altă abordare, prima dată “adun” toate erorile şi fac un singur exit.

public function setPerson($fname, $lname, $age)
{
  $this->error = array();

  if (empty($fname)) {
    $this->error["fname"] = "Missing first name";
  }

  if (empty($lname)) {
    $this->error["lname"] = "Missing last name";
  }

  if (empty($age) || !is_numeric($age) || $age < 0) {
    $this->error["age"] = "Invalid age given";
  }

  if(count($this->error))
    return false;

  $this->greeting = "Hello {$fname} of the clan {$lname}. Great to have a {$age} year old among us!";

  return true;

În felul asta afli toate erorile dintr-un foc, userul nu mai sta sa le afle prin încercare şi eroare.

1 Like