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

docs(core): document changes to autoRefresh #517

Merged
merged 3 commits into from
Mar 22, 2022
Merged

docs(core): document changes to autoRefresh #517

merged 3 commits into from
Mar 22, 2022

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Mar 22, 2022

Documentation for #512. The app-wide skipThrowForStatus is intentionally not documented because I don't anticipate CLI devs using that - they can do it in middleware if they'd like.

Did some other cleaning while I was in there.

@xavdid xavdid requested a review from eliangcs as a code owner March 22, 2022 00:01
eliangcs
eliangcs previously approved these changes Mar 22, 2022
Copy link
Member

@eliangcs eliangcs 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! I suggest we clarify on a paragraph.

In addition, I think we should also edit the v10 Breaking Change: Auth Refresh section, because it would look outdated when v12 is out. But that doesn't have to be in this PR. Your call.

10. call `response.throwForStatus()` unless `response.skipThrowForStatus` is `true`

The resulting response object is returned from `z.request()`.

#### Error Response Handling

Since v10, we call `response.throwForStatus()` before we return a response. You can prevent this by setting `skipThrowForStatus` on the request or response object. You can do this in `afterResponse` middleware if the API uses a status code >= 400 that should not be treated as an error.
Copy link
Member

Choose a reason for hiding this comment

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

I found this paragraph is bit confusing. I suggest we clarify:

Suggested change
Since v10, we call `response.throwForStatus()` before we return a response. You can prevent this by setting `skipThrowForStatus` on the request or response object. You can do this in `afterResponse` middleware if the API uses a status code >= 400 that should not be treated as an error.
Since v10, `z.request()` calls `response.throwForStatus()` before it returns a response. You can disable automatic error throwing by setting `skipThrowForStatus` on the request object:
// Disable automatic error throwing on the request object
const perform = async (z, bundle) => {
const response = await z.request({
url: '...',
skipThrowForStatus: true
});
// Now you handle error response on your own.
// The following is equivalent to response.throwForStatus().
if (response.status >= 400) {
throw new z.errors.ResponseError(response);
}
};
You can also do it in `afterResponse` if the API uses a status code >= 400 that should not be treated as an error.
// Don't throw an error when response status is 456
const disableAutoErrorThrowing = (response, z) => {
if (response.status === 456) {
response.skipThrowForStatus = true;
}
return response;
};
const App = {
// ...
afterResponse: [disableAutoErrorThrowing],
// ...
};

@xavdid
Copy link
Contributor Author

xavdid commented Mar 22, 2022

Ah, good call! So many sections.

I've gone ahead and pulled that whole section- it's not relevant in the 12.0 docs, but will live on in the older ones if we need it.

@xavdid xavdid merged commit 751de3b into master Mar 22, 2022
@xavdid xavdid deleted the PDE-3077 branch March 22, 2022 18:50
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.

3 participants