Added functionality to modify event#15
Conversation
| })}) | ||
|
|
||
| if (response.status === 403) { | ||
| throw new ApiPatchError("You do not have access to edit event information.") |
There was a problem hiding this comment.
this is an incorrect error. 403 doesnt have to mean "you do not have access". it could mean the token is not working. etc. Better respond with something more generic
There was a problem hiding this comment.
Got it, is this better?
You do not have access to edit event information. If you believe you are supposed to have access to this, try logging out and back in.
There was a problem hiding this comment.
No. "Access was denied. If you believe you are supposed to have access to this, try logging out and back in." is a lot better. Say what actually happened, not why you believe what happened happened. It makes troubleshooting harder
| throw new ApiPatchError("You do not have access to edit event information.") | ||
| } | ||
| else if (!response.ok) { | ||
| throw new ApiPatchError((await response.json())['error']); |
There was a problem hiding this comment.
also, we should prioritize reading the error over simply reacting to 403. Always prefer the error messsage from the server if it exists, as it will be more descriptive.
There was a problem hiding this comment.
Okay? In this scenario, if I prioritize !response.ok over response.status === 403, I get this:
SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data
Are you sure?
There was a problem hiding this comment.
In general it is better to then try/catch the json decode, than to hardcode the 403 response, imo.
But even better, the server should probably always return a json response. lets do that.
2 participants
No description provided.