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

IBX-8825: Added strict_mode for REST API #124

Merged
merged 1 commit into from
Sep 3, 2024
Merged

IBX-8825: Added strict_mode for REST API #124

merged 1 commit into from
Sep 3, 2024

Conversation

Steveb-p
Copy link
Contributor

@Steveb-p Steveb-p commented Aug 26, 2024

🎫 Issue IBX-8825

Description:

This PR introduces the concept of strict_mode for REST.

In strict_mode, when REST processing encounters an error/exception that it previously silenced (and usually logged as an error) it will re-throw it.

For QA:

Note

This functionality needs to be tested separately on prod and dev application environments.

Documentation:

Copy link

@Steveb-p Steveb-p changed the title RFC: Added strict_mode for REST API IBX-8825: Added strict_mode for REST API Aug 26, 2024
@Steveb-p Steveb-p marked this pull request as ready for review August 26, 2024 12:08
@Steveb-p Steveb-p requested a review from a team August 28, 2024 07:33
Copy link
Contributor

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

Looks good, but I would rename it to verbose_mode maybe or silence_mode: false. Strict to me isn't that self-explanatory but it's not a blocker.

@konradoboza konradoboza requested a review from a team August 28, 2024 07:38
@Steveb-p
Copy link
Contributor Author

Looks good, but I would rename it to verbose_mode maybe or silence_mode: false. Strict to me isn't that self-explanatory but it's not a blocker.

It follows a similar feature that is coming for Page Builder, and is available in Twig itself.

It's strict_mode because it converts logical errors that can be silenced on production into exceptions in dev environment. It matches PHP declare(strict_types=1); setting and JavaScripts "use strict".

Verbosity doesn't really match as we are not adding any additional info that wasn't already there. We're "promoting it" to a real exception.

@alongosz alongosz requested a review from a team August 28, 2024 10:57
@Steveb-p
Copy link
Contributor Author

Steveb-p commented Sep 2, 2024

@adamwojs I'd need a decision here whether QA should test this or not.

It might be quite difficult to test, since it requires some custom field types, and missing a normalizer for it's value. WDYT?

@adamwojs
Copy link
Member

adamwojs commented Sep 3, 2024

I'd need a decision here whether QA should test this or not. It might be quite difficult to test, since it requires some custom field types, and missing a normalizer for it's value. WDYT?

We can skip QA. It's doesn't affect prod. env.

@adamwojs adamwojs merged commit 397d3b5 into 4.6 Sep 3, 2024
14 checks passed
@adamwojs adamwojs deleted the strict-mode branch September 3, 2024 11:38
@Steveb-p
Copy link
Contributor Author

Steveb-p commented Sep 3, 2024

Merged up into main in 49a2bfd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants