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

feat: add debug-mode around all network requests #1239

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

Integralist
Copy link
Collaborator

There was a bug recently which we were only able to identify and fix, by dumping a related HTTP request/response.

We have --debug-mode but currently it only passes that information to go-fastly which dumps the req/resp.

This PR adds the same logic as found in go-fastly and ensures all network requests are wrapped in conditional branches checking for debug mode being enabled.

@Integralist Integralist added the enhancement New feature or request label Jul 4, 2024
@Integralist Integralist requested review from jsocol and kailan July 4, 2024 11:50
Copy link
Member

@kailan kailan left a comment

Choose a reason for hiding this comment

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

This is so helpful – thank you!

@Integralist
Copy link
Collaborator Author

Integralist commented Jul 4, 2024

NOTE: The tests that are failing in CI aren't fail locally 🤷🏻‍♂️

Nothing has changed with those files so not sure why they're failing now.

@jsocol if you have any ideas, then let me know. I'll try and find some time to dig into why later.

I checked the file that is checked by the test suite and nothing looks out of place:

[wasm-metadata]
build_info = "disable"
machine_info = "disable"
package_info = "disable"

@jsocol
Copy link
Member

jsocol commented Jul 8, 2024

We also got a failure in pkg/config/TestUseStatic reading config-legacy.toml which hasn't been touched in 2 years. Some kind of change to the MacOS runners recently?

@kpfleming
Copy link
Contributor

This was the tomlq change; I'll update this branch and the tests should pass.

@kpfleming
Copy link
Contributor

Tests are now passing, I'll merge this.

@kpfleming kpfleming merged commit 07aab14 into main Jul 9, 2024
8 checks passed
@kpfleming kpfleming deleted the integralist/debug-mode branch July 9, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants