Skip to content

Conversation

marcogrcr
Copy link
Contributor

WHAT?

Export ExpiredRequestException.

WHY?

So that users can:

import { ExpiredRequestException, verifyRequest } from '@contentful/node-apps-toolkit' try { verifyRequest(/* ...*/) } catch (e) { if (e instanceof ExpiredRequestException) { // expiration-specific logic } }

Instead of doing workarounds like:

import { verifyRequest } from '@contentful/node-apps-toolkit' import { ExpiredRequestException } from '@contentful/node-apps-toolkit/lib/requests/exceptions' // (...)

Or:

try { verifyRequest(/* ... */) } catch (e) { if (e && e.constructor && e.constructor.name === 'ExpiredRequestException') { // expiration-specific logic } }

Both of which rely on internal implementation details.

@marcogrcr marcogrcr requested a review from a team as a code owner August 23, 2024 20:05
WHAT? Export `ExpiredRequestException`. WHY? So that users can: ```ts import { ExpiredRequestException, verifyRequest } from '@contentful/node-apps-toolkit' try { verifyRequest(/* ...*/) } catch (e) { if (e instanceof ExpiredRequestException) { // expiration-specific logic } } ``` Instead of doing workarounds like: ```ts import { verifyRequest } from '@contentful/node-apps-toolkit' import { ExpiredRequestException } from '@contentful/node-apps-toolkit/lib/requests/exceptions' // (...) ``` Or: ```ts try { verifyRequest(/* ... */) } catch (e) { if (e && e.constructor && e.constructor.name === 'ExpiredRequestException') { // expiration-specific logic } } ``` Both of which rely on internal implementation details.
Copy link
Contributor

@jjolton-contentful jjolton-contentful left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@jjolton-contentful jjolton-contentful merged commit c4e1390 into contentful:main Aug 23, 2024
contentful-automation bot added a commit that referenced this pull request Aug 23, 2024
# [3.7.0](v3.6.1...v3.7.0) (2024-08-23) ### Features * export ExpiredRequestException ([#692](#692)) ([c4e1390](c4e1390))
@contentful-automation
Copy link
Contributor

🎉 This PR is included in version 3.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

marcogrcr added a commit to marcogrcr/node-apps-toolkit that referenced this pull request Aug 27, 2024
WHAT? Add `ValidationException` and throw it when `signRequest()` or `verifyRequest()` fail validation. WHY? When there's a validation error a `runtypes.ValidationError` is thrown. This is an internal implementation detail and as such, there is no supported way of detecting validation errors. Similar to [contentful#692], so that developers can: ```ts import { ValidationException, verifyRequest } from '@contentful/node-apps-toolkit' try { verifyRequest(/* ...*/) } catch (e) { if (e instanceof ValidationException) { if (e.constraintName === 'SecretLength') { // this is unexpected, return 500 status code and alarm } else { // this is expected, should return 400 or 403 status code } } } ``` [contentful#692]: contentful#692
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants