-
Notifications
You must be signed in to change notification settings - Fork 11
Error handling #275
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
Error handling #275
Conversation
|
FYI @joostfarla ik heb geforce-pusht naar zowel de base branch als jouw branch om de build te laten slagen. Helaas hebben we nog geen preview hiervan, want dat draait vooralsnog alleen op branches van Logius-standaarden repositories. Daar ga ik nog naar kijken. |
|
Nee helaas dat gaat niet werken, omdat de |
TimvdLippe
left a comment
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.
Goeie stap! Over de velden is nog wat onduidelijkheid, maar lijkt mij een goede richting dat dit op gaat.
| <ul> | ||
| <li>For JSON request bodies: an object with a <code>pointer</code> property containing a JSON Pointer [[rfc6901]] expression pointing to the value.</li> | ||
| <li>For XML request bodies: an element with a <code>path</code> attribute containing an absolute XPath v3.1 [[xpath-31]] expression pointing to the value.</li> | ||
| <li>For query parameters: an object with a <code>name</code> property (the parameter name). When the same name appears multiple times, include an <code>index</code> property, containing a zero-based index position.</li> |
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.
Ik zou de index altijd includen. Ook al is het er 1, dan is het in ieder geval duidelijk dat de index 0 is.
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.
Multi-valued query parameters zijn zeldzaam in de praktijk. Vandaar was mijn voorstel om index alleen op te nemen voor de uitzonderingsgevallen (om bloating te voorkomen). Je zou kunnen stellen dat een index voor single-valued parameters niet van toepassing is.
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.
Persoonlijk vind ik duidelijkheid belangrijker dan bloating bij errors. Zeker omdat errors niet zo vaak voorkomen, zie ik bloating minder als een factor. Daarom zou mijn voorkeur zijn om consistentie na te streven, ook in het kader van het checken of het schema wel voldoet. Mochten anderen daar toch anders over denken, dan kan dat natuurlijk ook. My 2 cents.
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.
Bij POST/PUT komen (meerdere) errors denk ik vaak genoeg voor. Waar we misschien nog wel iets over zouden moeten zeggen is of servers een request body volledig zouden moeten evalueren (alle properties valideren) ipv stoppen met valideren na de eerste fout. In het laatste geval krijg je ping-pong communicatie, wat niet heel effifiënt is. Of we besluiten dit open te laten, maar hoe dan ook goed om hier een statement over op te nemen. Heb het toegevoegd aan de TBD-punten in de description van dit PR.
Co-authored-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
|
Is het goed om deze PR te mergen in de hoofd PR en daar de discussie verder te houden? Of wil je liever de TBD eerst nog oplossen? Mij maakt de volgorde minder uit, zolang de discussie maar op 1 plek plaats vindt. |
Merge maar, zolang je de TBDs ook mee overhevelt naar het andere issue 👍 |
TBD: