Skip to content

Report http response errors during upoad.#223

Merged
zhming0 merged 1 commit intomainfrom
ming/pie-2813
Sep 17, 2024
Merged

Report http response errors during upoad.#223
zhming0 merged 1 commit intomainfrom
ming/pie-2813

Conversation

@zhming0
Copy link
Contributor

@zhming0 zhming0 commented Sep 16, 2024 •
edited
Loading

part of PIE-2813

Fix uploader so it will throw a runtime error if backend does not return 2xx.
This will result in a log message if any upload fails.

Note it will not fail a build if the upload fails.

@zhming0 zhming0 requested a review from a team September 16, 2024 06:09
@wooly
Copy link

wooly commented Sep 16, 2024

I'm not super keen on this, since it will cause people's builds to fail if the upload API is down. I always thought of the test collectors as a best-effort service.

What do you think about making this a configuration option so people could turn it on if they so wished?
How does this behaviour compare to the other collectors? Do they raise an issue if the data cannot be sent?

@zhming0
Copy link
Contributor Author

zhming0 commented Sep 16, 2024

I'm not super keen on this, since it will cause people's builds to fail if the upload API is down. I always thought of the test collectors as a best-effort service.

@wooly and @nprizal

It won't fail people's build. Our uploader has logic capturing the error.

It was just previously, the error reporting was never working, so we were completely in the dark about what happen in the collector.

gchan
gchan approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@gchan gchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, have we considered adding a tests to verify this new functionality? I assume you have manually tested this and viewed the new log message.

@wooly
Copy link

wooly commented Sep 16, 2024

@zhming0 ah yep I see it now, sorry for the noise!

@zhming0
Copy link
Contributor Author

zhming0 commented Sep 17, 2024

@gchan I added some tests. 👍🏿

@zhming0 zhming0 merged commit fbb2dec into main Sep 17, 2024
@zhming0 zhming0 deleted the ming/pie-2813 branch September 17, 2024 00:22
@zhming0 zhming0 changed the title Handle http response errors during upoad. Report http response errors during upoad. Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gchan gchan gchan approved these changes

@KatieWright26 KatieWright26 KatieWright26 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zhming0 @wooly @gchan @KatieWright26