Skip to content

Update README.md#645

Merged
abecevello merged 1 commit intoShopify:mainfrom
jonp200:patch-1
May 28, 2025
Merged

Update README.md#645
abecevello merged 1 commit intoShopify:mainfrom
jonp200:patch-1

Conversation

@jonp200
Copy link
Copy Markdown
Contributor

@jonp200 jonp200 commented May 28, 2025

While using the Go client (github.com/Shopify/toxiproxy/client), I found that after creating a proxy and adding toxics via proxy.AddToxic(...), the toxic does not take effect unless I explicitly call proxy.Save() afterward.

This is not currently mentioned in the documentation or examples, and it caused some confusion when toxics weren’t being applied despite no errors during creation.

Example:

proxy, _ := client.CreateProxy("my_proxy", "localhost:5000", "localhost:6000")
proxy.AddToxic("latency_downstream", "latency", "downstream", 1.0, client.Attributes{
    "latency": 100,
})
proxy.Save() // Without this, the toxic is not active

Suggestions:

  • Mention this requirement in the README and/or inline GoDoc.
  • Possibly make AddToxic call Save() internally, or clarify the design choice if not.

While using the Go client (`github.com/Shopify/toxiproxy/client`), I found that after creating a proxy and adding toxics via `proxy.AddToxic(...)`, the toxic does not take effect unless I explicitly call `proxy.Save()` afterward.

This is not currently mentioned in the documentation or examples, and it caused some confusion when toxics weren’t being applied despite no errors during creation.

**Suggestions:**

- Mention this requirement in the README and/or inline GoDoc.
- Possibly make `AddToxic` call `Save()` internally, or clarify the design choice if not.
@jonp200
Copy link
Copy Markdown
Contributor Author

jonp200 commented May 28, 2025

I have signed the CLA!

Copy link
Copy Markdown
Member

@abecevello abecevello left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the contribution.

@abecevello abecevello merged commit 79ca42e into Shopify:main May 28, 2025
6 checks passed
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.

2 participants