-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add new error mapping for http error code 403 and 404 #1457
Conversation
@@ -171,6 +171,8 @@ typedef NS_ENUM(NSInteger, MSIDErrorCode) | |||
*/ |
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.
This pull request does not update changelog.txt.
Please consider if this change would be noticeable to a partner or user and either update changelog.txt or resolve this conversation.
IdentityCore/src/MSIDError.h
Outdated
@@ -171,6 +171,8 @@ typedef NS_ENUM(NSInteger, MSIDErrorCode) | |||
*/ | |||
|
|||
MSIDErrorServerUnhandledResponse = -51500, | |||
// http status Code 403 or 404 | |||
MSIDErrorUnExpectedHttpResponse = -51501, |
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.
nit: typo unexpected
MSIDErrorUnExpectedHttpResponse = -51501, | |
MSIDErrorUnexpectedHttpResponse = -51501, |
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.
Fixed
httpUnderlyingError = MSIDCreateError(MSIDHttpErrorCodeDomain, MSIDErrorUnExpectedHttpResponse, errorDescription, nil, nil, nil, context.correlationId, nil, YES); | ||
} | ||
|
||
NSError *httpError = MSIDCreateError(MSIDHttpErrorCodeDomain, MSIDErrorServerUnhandledResponse, errorDescription, nil, nil, httpUnderlyingError, context.correlationId, additionalInfo, YES); |
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.
Question, should the errorCode remain as MSIDErrorServerUnhandledResponse
, since the MSIDErrorUnexpectedHttpResponse
is now added as underlying error code?
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.
I didnt change it because we want to keep the top level error same.
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.
Oh yeah, you are right, I got confused.
Proposed changes
When token request fails with http error code 403 and 404 , we currently return MSIDErrorServerUnHandledResponse. This PR will add a new error MSIDErrorUnexpectedHttpResponse in case of http error code 403 or 404 and add this as an underlying error in the MSIDErrorServerUnHandledResponse error returned.
Type of change
Risk
Additional information