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): Load errorLog regardless of credential refresh result #3103

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

nalanj
Copy link
Contributor

@nalanj nalanj commented Dec 3, 2024

When viewing a connection we had a strange state that was possible where there was an auth error on a connection but because it was not a connection that could be refreshed it wasn't showing the error in the UI once viewing the settings.

See https://linear.app/nango/issue/NAN-2255/authorization-tab-for-connections-sometimes-doesnt-show-auth-errors

How I tested it

I set up an auth error row in my test db on a connection that didn't refresh (I used Unauthenticated) and confirmed that it displayed correctly. Here's a sample insert (you'll need to tweak it to get it to work with a connection you have set up):

INSERT INTO "nango"."_nango_active_logs"("id","type","action","connection_id","log_id","active","sync_id","created_at","updated_at")
VALUES
(391,E'auth',E'token_refresh',73,E'begpVdnwQLenQDHNVicf',TRUE,NULL,E'2024-11-26 16:53:33.628375+00',E'2024-11-26 16:53:33.628375+00');

Before this change you would see the auth error in the connections list, but visiting the connection would not show a dot on the auth tab or show an error on that page. After this change you'll see the dot and the error.

@nalanj nalanj self-assigned this Dec 3, 2024
Copy link

linear bot commented Dec 3, 2024

@nalanj nalanj marked this pull request as ready for review December 3, 2024 13:26
@nalanj nalanj requested a review from a team December 3, 2024 13:27
Copy link
Collaborator

@bodinsamuel bodinsamuel left a comment

Choose a reason for hiding this comment

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

Indeed, the whole get and/or refresh credentials situation is really a pain to manage

@nalanj nalanj merged commit 5608eaa into master Dec 3, 2024
22 checks passed
@nalanj nalanj deleted the alan/nan-2255 branch December 3, 2024 14:11
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