Skip to content
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

Relays upload request errors via nsnotifications #1023

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

GabePires
Copy link

What does this PR do?
Segment posts a notification if the segment request fails.
However, this notification doesn't say what the error is.
This PR relays the error information to that notification.

Where should the reviewer start?
Please take a look at the changes made in this function first

- (nullable NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry, NSError * _Nullable error))completionHandler

I've added NSError * _Nullable error to the completion handler block.

Next, please take a look at the changes made in this function:

- (void)sendData:(NSArray *)batch

How should this be manually tested?
Use Charles Proxy or another tool to fail the /batch request with different response statuses.

Any background context you want to provide?
Sometimes, we're seeing the requests fail, but don't know why. Therefore, we are trying to add some logging to find out why these requests are failing.

What are the relevant tickets?
-none-

Screenshots or screencasts (if UI/UX change)
-none-

Questions:

  • Does the docs need an update? Yes. Someone should update how to capture these request errors.
  • Are there any security concerns? No
  • Do we need to update engineering / success? No

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant