feature/SCKT-3_Adding_error_messages_object_for_422_status_code #4
Conversation
MostafaFawzy7
requested review from
Abdallah-Abdelkhalek,
HassanElZarkawy,
MMGAbdelhay and
SirNarsh
src/ApiKitClient.ts
Outdated
| if (status === 401 && this.unauthorizationCallback) { | ||
| this.unauthorizationCallback(); | ||
| } | ||
| if (this.globalErrorCallback) { |
There was a problem hiding this comment.
Yes you are right, I started with like this at 1st but I changed it accidentally later!
src/ApiKitClient.ts
Outdated
| this.unauthorizationCallback(); | ||
| } | ||
| if (this.globalErrorCallback) { | ||
| this.globalErrorCallback(); |
There was a problem hiding this comment.
you might need to pass the error object
There was a problem hiding this comment.
Didn't get passing error object where...?
src/ApiKitClient.ts
Outdated
| if (status === 401 && this.unauthorizationCallback) { | ||
| this.unauthorizationCallback(); | ||
| } else if (this.globalErrorCallback) { | ||
| this.globalErrorCallback(); |
There was a problem hiding this comment.
Like this
| this.globalErrorCallback(); | |
| this.globalErrorCallback(error); |
There was a problem hiding this comment.
This is a callback doesn't accept any parameter why should I pass the error object to it?!
There was a problem hiding this comment.
@MostafaFawzy7 to let the consumer know what exactly happened. Give the consuming app some context. This is actually a pretty good idea.
There was a problem hiding this comment.
@HassanElZarkawy The idea of this callback is to add a custom error by the consumer at the global scope of error handling, why to pass to it the error object which is already exposed by the Promise rejection at the next line of this condition?
|
Did you test it locally by linking? |
Why? Do you have an issue with it? it's just a condition to invoke the new callback... |
I didn't test it and was asking if you did |
@SirNarsh @HassanElZarkawy @amrography Done as we agreed on this finally on the standup! |
No description provided.