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

fix(server): /connections/:connectionId New Format #3200

Merged
merged 5 commits into from
Dec 19, 2024
Merged

Conversation

nalanj
Copy link
Contributor

@nalanj nalanj commented Dec 19, 2024

This is another attempt to get the new /connections/:connectionId PR to land, with some updates to remove the status code changes the previous attempts included. That should help us see how much we've actually changed.

@nalanj nalanj self-assigned this Dec 19, 2024
onRefreshFailed: connectionRefreshFailedHook
});
if (credentialResponse.isErr()) {
res.status(credentialResponse.error.status).send({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🍖 the core of this update

Copy link
Collaborator

Choose a reason for hiding this comment

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

are we sure credentialResponse.error.status is always defined, if not let's || 5XX

@nalanj nalanj marked this pull request as ready for review December 19, 2024 16:21
@nalanj nalanj requested a review from a team December 19, 2024 16:29
Copy link
Collaborator

@TBonnin TBonnin left a comment

Choose a reason for hiding this comment

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

3rd time the charm

onRefreshFailed: connectionRefreshFailedHook
});
if (credentialResponse.isErr()) {
res.status(credentialResponse.error.status).send({
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we sure credentialResponse.error.status is always defined, if not let's || 5XX

@nalanj
Copy link
Contributor Author

nalanj commented Dec 19, 2024

@TBonnin NangoError always has status set, and the credentialResponse error type is NangoError, so I think we're safe.

@nalanj nalanj merged commit 792467a into master Dec 19, 2024
21 checks passed
@nalanj nalanj deleted the alan/get-connection branch December 19, 2024 18:42
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