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

Unify WebBrowser API in regards to nullable bodies #2593

Merged
merged 3 commits into from Jun 1, 2022
Merged

Conversation

JustArchi
Copy link
Member

Commit 6178b12 introduced two new request options which require response content to be nullable to account for it not being parsed.

While the above change was non-breaking, it further complicates ASF logic which now has two functions that do exactly the same apart from one small detail and different response type.

It makes sense to unify those two, and while at it bullet-proof for similar problems with HtmlDocumentResponse where we also might need to account for null/empty HTML on error.

This actually is not a breaking change per-se. Warnings will be emitted for plugin creators, but old code should still work fine without recompilation, as it was just annotation change. In terms of usage, null body might be returned only when explicitly asking for it with new request options, so even previous use cases remain valid.

I'm postponing it until next release cycle.

@JustArchi JustArchi added ✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 📢 Feedback welcome Issues marked with this label are open to any potential feedback that could help us. labels May 28, 2022
@JustArchi JustArchi requested a review from Abrynos May 28, 2022 19:46
Copy link
Contributor

@Abrynos Abrynos left a comment

Choose a reason for hiding this comment

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

I trust you found all the code where we check response.Content and needed to add either the ? or the ! operator, because I'm not searching for those 😅

ArchiSteamFarm/Web/Responses/ObjectResponse.cs Outdated Show resolved Hide resolved
@JustArchi JustArchi merged commit 715ed03 into main Jun 1, 2022
@JustArchi JustArchi deleted the nullable-bodies branch June 1, 2022 19:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 📢 Feedback welcome Issues marked with this label are open to any potential feedback that could help us.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants