[TT-12193] Fix poor error handling in webhook event templates by titpetric · Pull Request #6303 · TykTechnologies/tyk

titpetric · 2024-05-23T07:19:23Z

User description

Fixes error handling in webhook handler templates

https://tyktech.atlassian.net/browse/TT-12193

PR Type

Bug fix, Tests

Description

Added error handling for template execution in the CreateBody method of WebHookHandler.
Refactored test helper function to include error handling.
Replaced panic and error checks with assert.NoError in tests.
Improved error messages in tests for better clarity.

Changes walkthrough 📝

Relevant files

Bug fix

event_handler_webhooks.go Add error handling in `CreateBody` method of WebHookHandler gateway/event_handler_webhooks.go Added error handling for template execution in `CreateBody` method.	+4/-1

Tests

event_handler_webhooks_test.go `Improve error handling and assertions in webhook handler tests` gateway/event_handler_webhooks_test.go Refactored test helper function to include error handling. Replaced panic and error checks with `assert.NoError`. Improved error messages in tests.	+22/-24

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions · 2024-05-23T07:19:59Z

PR Description updated to latest commit (fe24bf0)

github-actions · 2024-05-23T07:20:11Z

API Changes

no api changes detected

github-actions · 2024-05-23T07:20:14Z

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and localized to specific methods, involving basic error handling and test refactoring.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: The error handling in `CreateBody` might not log the error, which could make debugging more difficult if the template execution fails.
🔒 Security concerns	No

Code feedback:

relevant file	gateway/event_handler_webhooks.go
suggestion	Consider logging the error before returning it in the `CreateBody` method. This can help in debugging issues related to template execution failures. [important]
relevant line	return "", err

relevant file	gateway/event_handler_webhooks_test.go
suggestion	Use more descriptive error messages in your tests to help identify issues quickly during failures. For example, in `TestCreateBody`, specify what was expected and what was actually received. [medium]
relevant line	t.Errorf("Body incorrect, is: %q", body)

github-actions · 2024-05-23T07:20:46Z

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Add a detailed error message for template execution failure Consider adding a detailed error message when the template execution fails. This can help with debugging and understanding the context of the error. gateway/event_handler_webhooks.go [204-205] if err := w.template.Execute(&reqBody, em); err != nil { - return "", err + return "", fmt.Errorf("template execution failed: %w", err) } Suggestion importance[1-10]: 9 Why: Adding a detailed error message when the template execution fails can significantly aid in debugging and understanding the context of the error. This is a valuable enhancement for error handling.	9
Enhancement	Use descriptive error messages in test assertions Use a more descriptive error message in the test assertion to provide clarity on what the expected outcome should be. gateway/event_handler_webhooks_test.go [178] -t.Errorf("Body incorrect, is: %q", body) +t.Errorf("Body incorrect, expected to contain %q, got: %q", expectedBody, body) Suggestion importance[1-10]: 8 Why: Using a more descriptive error message in the test assertion provides clarity on what the expected outcome should be, which improves the readability and maintainability of the test code.	8
Best practice	Improve error handling in tests for better error reporting Replace the direct panic on error with a more controlled error handling approach in tests to avoid abrupt test exits and allow for better error reporting. gateway/event_handler_webhooks_test.go [25] -assert.NoError(t, err) +if err != nil { + t.Errorf("Unexpected error: %v", err) +} Suggestion importance[1-10]: 7 Why: Replacing the direct panic on error with a more controlled error handling approach in tests can avoid abrupt test exits and provide better error reporting. However, the existing use of `assert.NoError` is already a common practice in Go tests.	7
Best practice	Explicitly check for errors in tests for detailed error handling Consider checking for errors explicitly in tests instead of using assert.NoError to ensure that all errors are properly handled and logged. gateway/event_handler_webhooks_test.go [199] -assert.NoError(t, err) +if err != nil { + t.Errorf("Error while creating body: %v", err) +} Suggestion importance[1-10]: 7 Why: Explicitly checking for errors instead of using `assert.NoError` ensures that all errors are properly handled and logged. This can provide more detailed error information, although `assert.NoError` is already a standard practice in Go tests.	7

sonarqubecloud · 2024-05-23T07:32:35Z

Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

titpetric · 2024-05-23T08:26:42Z

/release to release-5.3

tykbot · 2024-05-23T08:26:58Z

Working on it! Note that it can take a few minutes.

### **User description** Fixes error handling in webhook handler templates https://tyktech.atlassian.net/browse/TT-12193 ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Added error handling for template execution in the `CreateBody` method of `WebHookHandler`. - Refactored test helper function to include error handling. - Replaced panic and error checks with `assert.NoError` in tests. - Improved error messages in tests for better clarity. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks.go</strong><dd><code>Add error handling in `CreateBody` method of WebHookHandler</code></dd></summary> <hr> gateway/event_handler_webhooks.go <li>Added error handling for template execution in <code>CreateBody</code> method.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-6587ad3f2629cfa6c84a71144127acd6cc7824e5141f0b4961848945a87e0198">+4/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Tests </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks_test.go</strong><dd><code>Improve error handling and assertions in webhook handler tests</code></dd></summary> <hr> gateway/event_handler_webhooks_test.go <li>Refactored test helper function to include error handling.<br> <li> Replaced panic and error checks with <code>assert.NoError</code>.<br> <li> Improved error messages in tests.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-1a2f8d6ab4443031b0f8486809453f6389291a6f625745fe8c65ac5cdb0e9621">+22/-24</a>  </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions Co-authored-by: Tit Petric <[email protected]> (cherry picked from commit 11d5798)

tykbot · 2024-05-23T08:27:16Z

@titpetric Succesfully merged PR

… event templates (#6303) [TT-12193] Fix poor error handling in webhook event templates (#6303) ### **User description** Fixes error handling in webhook handler templates https://tyktech.atlassian.net/browse/TT-12193 ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Added error handling for template execution in the `CreateBody` method of `WebHookHandler`. - Refactored test helper function to include error handling. - Replaced panic and error checks with `assert.NoError` in tests. - Improved error messages in tests for better clarity. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks.go</strong><dd><code>Add error handling in `CreateBody` method of WebHookHandler</code></dd></summary> <hr> gateway/event_handler_webhooks.go <li>Added error handling for template execution in <code>CreateBody</code> method.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-6587ad3f2629cfa6c84a71144127acd6cc7824e5141f0b4961848945a87e0198">+4/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Tests </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks_test.go</strong><dd><code>Improve error handling and assertions in webhook handler tests</code></dd></summary> <hr> gateway/event_handler_webhooks_test.go <li>Refactored test helper function to include error handling.<br> <li> Replaced panic and error checks with <code>assert.NoError</code>.<br> <li> Improved error messages in tests.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-1a2f8d6ab4443031b0f8486809453f6389291a6f625745fe8c65ac5cdb0e9621">+22/-24</a>  </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions Co-authored-by: Tit Petric <[email protected]>

### **User description** Fixes error handling in webhook handler templates https://tyktech.atlassian.net/browse/TT-12193 ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Added error handling for template execution in the `CreateBody` method of `WebHookHandler`. - Refactored test helper function to include error handling. - Replaced panic and error checks with `assert.NoError` in tests. - Improved error messages in tests for better clarity. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks.go</strong><dd><code>Add error handling in `CreateBody` method of WebHookHandler</code></dd></summary> <hr> gateway/event_handler_webhooks.go <li>Added error handling for template execution in <code>CreateBody</code> method.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-6587ad3f2629cfa6c84a71144127acd6cc7824e5141f0b4961848945a87e0198">+4/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Tests </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks_test.go</strong><dd><code>Improve error handling and assertions in webhook handler tests</code></dd></summary> <hr> gateway/event_handler_webhooks_test.go <li>Refactored test helper function to include error handling.<br> <li> Replaced panic and error checks with <code>assert.NoError</code>.<br> <li> Improved error messages in tests.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-1a2f8d6ab4443031b0f8486809453f6389291a6f625745fe8c65ac5cdb0e9621">+22/-24</a>  </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions Co-authored-by: Tit Petric <[email protected]>

… event templates (#6303) [TT-12193] Fix poor error handling in webhook event templates (#6303) ### **User description** Fixes error handling in webhook handler templates https://tyktech.atlassian.net/browse/TT-12193 ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Added error handling for template execution in the `CreateBody` method of `WebHookHandler`. - Refactored test helper function to include error handling. - Replaced panic and error checks with `assert.NoError` in tests. - Improved error messages in tests for better clarity. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks.go</strong><dd><code>Add error handling in `CreateBody` method of WebHookHandler</code></dd></summary> <hr> gateway/event_handler_webhooks.go <li>Added error handling for template execution in <code>CreateBody</code> method.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-6587ad3f2629cfa6c84a71144127acd6cc7824e5141f0b4961848945a87e0198">+4/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Tests </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks_test.go</strong><dd><code>Improve error handling and assertions in webhook handler tests</code></dd></summary> <hr> gateway/event_handler_webhooks_test.go <li>Refactored test helper function to include error handling.<br> <li> Replaced panic and error checks with <code>assert.NoError</code>.<br> <li> Improved error messages in tests.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-1a2f8d6ab4443031b0f8486809453f6389291a6f625745fe8c65ac5cdb0e9621">+22/-24</a>  </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions Co-authored-by: Tit Petric <[email protected]>

… event templates (TykTechnologies#6303) [TT-12193] Fix poor error handling in webhook event templates (TykTechnologies#6303) ### **User description** Fixes error handling in webhook handler templates https://tyktech.atlassian.net/browse/TT-12193 ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Added error handling for template execution in the `CreateBody` method of `WebHookHandler`. - Refactored test helper function to include error handling. - Replaced panic and error checks with `assert.NoError` in tests. - Improved error messages in tests for better clarity. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks.go</strong><dd><code>Add error handling in `CreateBody` method of WebHookHandler</code></dd></summary> <hr> gateway/event_handler_webhooks.go <li>Added error handling for template execution in <code>CreateBody</code> method.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-6587ad3f2629cfa6c84a71144127acd6cc7824e5141f0b4961848945a87e0198">+4/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Tests </strong></td><td><table> <tr> <td> <details> <summary><strong>event_handler_webhooks_test.go</strong><dd><code>Improve error handling and assertions in webhook handler tests</code></dd></summary> <hr> gateway/event_handler_webhooks_test.go <li>Refactored test helper function to include error handling.<br> <li> Replaced panic and error checks with <code>assert.NoError</code>.<br> <li> Improved error messages in tests.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6303/files#diff-1a2f8d6ab4443031b0f8486809453f6389291a6f625745fe8c65ac5cdb0e9621">+22/-24</a>  </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions Co-authored-by: Tit Petric <[email protected]>

Fix poor error handling in webhook event templates

fe24bf0

titpetric requested a review from andrei-tyk May 23, 2024 07:19

github-actions Bot added Tests Bug fix labels May 23, 2024

github-actions Bot added the Review effort [1-5]: 2 label May 23, 2024

titpetric changed the title ~~Fix poor error handling in webhook event templates~~ [TT-12193] Fix poor error handling in webhook event templates May 23, 2024

andrei-tyk approved these changes May 23, 2024

View reviewed changes

titpetric enabled auto-merge (squash) May 23, 2024 08:10

titpetric merged commit 11d5798 into master May 23, 2024

titpetric deleted the fix/event-handler-webhooks-error-handling branch May 23, 2024 08:24

buger mentioned this pull request May 23, 2024

Merging to release-5.3: [TT-12193] Fix poor error handling in webhook event templates (#6303) #6305

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[TT-12193] Fix poor error handling in webhook event templates#6303

[TT-12193] Fix poor error handling in webhook event templates#6303
titpetric merged 1 commit into
masterfrom
fix/event-handler-webhooks-error-handling

titpetric commented May 23, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented May 23, 2024

Uh oh!

github-actions Bot commented May 23, 2024

Uh oh!

github-actions Bot commented May 23, 2024

Uh oh!

github-actions Bot commented May 23, 2024

Uh oh!

sonarqubecloud Bot commented May 23, 2024

Uh oh!

titpetric commented May 23, 2024

Uh oh!

tykbot Bot commented May 23, 2024

Uh oh!

tykbot Bot commented May 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

titpetric commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

Changes walkthrough 📝

Uh oh!

github-actions Bot commented May 23, 2024

Uh oh!

github-actions Bot commented May 23, 2024

Uh oh!

github-actions Bot commented May 23, 2024

PR Review 🔍

Uh oh!

github-actions Bot commented May 23, 2024

PR Code Suggestions ✨

Uh oh!

sonarqubecloud Bot commented May 23, 2024

Quality Gate failed

Uh oh!

titpetric commented May 23, 2024

Uh oh!

tykbot Bot commented May 23, 2024

Uh oh!

tykbot Bot commented May 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

titpetric commented May 23, 2024 •

edited

Loading