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

Dashboards: Fix issue causing crashes when saving new dashboard #77620

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

kaydelaney
Copy link
Contributor

Maybe "avoids issue" is a better title. The underlying issue is here:

useEffect(() => {
if (error.data && proxyHandlesError(error.data.status)) {
error.isHandled = true;
}
}, [error]);

Specifically line 34 where error.isHandled is set to true.

It seems like what happens is:

  1. User creates new dashboard
  2. User adds panel, edit view is opened
  3. User hits save, drawer opens, then confirms save
  4. POST request is made with dashboard json, is saved successfully
  5. After save, tries to exit panel edit view
  6. DashboardPrompt onHistoryBlock function is called.
  7. hasChanges(dashboard, original) is called, but at this stage original still has version 0, while dashboard has version 1. Note that there is code in DashboardPrompt to update original when a DashboardSavedEvent is emitted, but I guess hasChanges gets executed first
  8. Unsaved changes prompt appears, user hits save
  9. POST request is made with the same dashboard json, error gets returned since it already exists
  10. Line 34 in SaveDashboardErrorProxy occurs, causing the crash.

So with this PR the issue is sort of avoided by saying "ignore changes if all we're changing is a dashboard that hasn't been saved yet", but I guess a "real" fix would figure out how to update original in DashboardPrompt before it looks for unsaved changes.

Closes #77593

Copy link
Member

@dprokop dprokop left a comment

Choose a reason for hiding this comment

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

Confirm - this is fixing the problem :) It's just some tests that will require updating.

@kaydelaney kaydelaney enabled auto-merge (squash) November 3, 2023 16:24
@kaydelaney kaydelaney merged commit c98add6 into main Nov 3, 2023
16 checks passed
@kaydelaney kaydelaney deleted the issue-77593 branch November 3, 2023 16:34
grafana-delivery-bot bot pushed a commit that referenced this pull request Nov 3, 2023
dprokop added a commit that referenced this pull request Nov 13, 2023
…oard (#77641)

* Dashboards: Fix issue causing crashes when saving new dashboard (#77620)

Closes #77593

(cherry picked from commit c98add6)

* Test fix

---------

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>
Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Dashboard: Page crashing when saving a new dashboard
3 participants