[Dashboard] Fix error handling in "Save to library" modal for Links panel by olapawlus · Pull Request #231168 · elastic/kibana

olapawlus · 2025-08-08T14:37:51Z

When saving a Links panel to the library from the Dashboard, errors during the save process are not properly caught or displayed to the user.

This PR fixes the error handling and ensures that an error toast is shown to the user.

Video Demonstration of the fix:

Screen.Recording.2025-08-08.at.16.14.58.mov

src/platform/plugins/private/links/public/components/editor/links_editor.tsx

elasticmachine · 2025-08-08T19:41:18Z

Pinging @elastic/kibana-presentation (Team:Presentation)

nreese

Thanks for opening this issue and a PR to resolve the issue. I think the current solution resolves the issue too high in the call stack, and leaves other consumers of showSaveModal vulnerable to the same problem. For example, if you try to save a map with no network connect, the save model hangs just like this issue.

So instead of solving the problem for one instance in src/platform/plugins/private/links/public/content_management/save_to_library.tsx, lets solve the problem for all users and make showSaveModal more robust by putting the try/catch around onSave in src/platform/plugins/shared/saved_objects/public/save_modal/show_saved_object_save_modal.tsx

const onSaveConfirmed: MinimalSaveModalProps['onSave'] = async (...args) => {
        try {
          const response = await onSave(...args);
          // close modal if we either hit an error or the saved object got an id
          if (Boolean(isSuccess(response) ? response.id : response.error)) {
            closeModal();
          }
          return response;
        } catch(error) {
          closeModal();
          return { error };
        }
      };

Then links only needs one change, in src/platform/plugins/private/links/public/editor/get_editor_flyout.tsx, where runSaveToLibrary is called. onSaveToLibrary should throw if saveResult.error

onSaveToLibrary={async (newLinks: ResolvedLink[], newLayout: LinksLayoutType) => {
        const newState = {
          ...initialState,
          links: newLinks,
          layout: newLayout,
        };
        if (initialState?.savedObjectId) {
          const { savedObjectId, ...updateState } = newState;
          await linksClient.update({
            id: initialState.savedObjectId,
            data: {
              ...updateState,
              links: serializeResolvedLinks(newLinks),
            },
            options: { references: [] },
          });
          onCompleteEdit?.(newState);
          closeFlyout();
        } else {
          const saveResult = await runSaveToLibrary(newState);
          if (saveResult?.error) throw saveResult.error;
          onCompleteEdit?.(saveResult);
          // If saveResult is undefined, the user cancelled the save as modal and we should not close the flyout
          if (saveResult) closeFlyout();
        }
      }}

Then if wanted, try/catch could be removed from src/platform/plugins/private/links/public/content_management/save_to_library.tsx

olapawlus · 2025-08-11T14:37:55Z

Thanks for opening this issue and a PR to resolve the issue. I think the current solution resolves the issue too high in the call stack, and leaves other consumers of showSaveModal vulnerable to the same problem. For example, if you try to save a map with no network connect, the save model hangs just like this issue.

So instead of solving the problem for one instance in src/platform/plugins/private/links/public/content_management/save_to_library.tsx, lets solve the problem for all users and make showSaveModal more robust by putting the try/catch around onSave in src/platform/plugins/shared/saved_objects/public/save_modal/show_saved_object_save_modal.tsx
const onSaveConfirmed: MinimalSaveModalProps['onSave'] = async (...args) => {
        try {
          const response = await onSave(...args);
          // close modal if we either hit an error or the saved object got an id
          if (Boolean(isSuccess(response) ? response.id : response.error)) {
            closeModal();
          }
          return response;
        } catch(error) {
          closeModal();
          return { error };
        }
      };
Then links only needs one change, in src/platform/plugins/private/links/public/editor/get_editor_flyout.tsx, where runSaveToLibrary is called. onSaveToLibrary should throw if saveResult.error
onSaveToLibrary={async (newLinks: ResolvedLink[], newLayout: LinksLayoutType) => {
        const newState = {
          ...initialState,
          links: newLinks,
          layout: newLayout,
        };
        if (initialState?.savedObjectId) {
          const { savedObjectId, ...updateState } = newState;
          await linksClient.update({
            id: initialState.savedObjectId,
            data: {
              ...updateState,
              links: serializeResolvedLinks(newLinks),
            },
            options: { references: [] },
          });
          onCompleteEdit?.(newState);
          closeFlyout();
        } else {
          const saveResult = await runSaveToLibrary(newState);
          if (saveResult?.error) throw saveResult.error;
          onCompleteEdit?.(saveResult);
          // If saveResult is undefined, the user cancelled the save as modal and we should not close the flyout
          if (saveResult) closeFlyout();
        }
      }}
Then if wanted, try/catch could be removed from src/platform/plugins/private/links/public/content_management/save_to_library.tsx

Thanks so much for the really helpful explanation of the pattern with returning { error } and then throwing it higher up in the code!
I also added the same kind of handling in the checkForDuplicateTitle, because before the toast wasn’t showing if the error happened during the checkForDuplicateTitle function used in saveToLibrary. The toast also didn’t show when I removed the try-catch from saveToLibrary. Now the toast shows correctly.

elasticmachine · 2025-08-11T16:24:21Z

💔 Build Failed

Buildkite Build
Commit: 92263e8

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should call batchBackfillRuleGaps correctly
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should call batchBackfillRuleGaps correctly
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should ensure the user is authorized for each combination of ruleTypeId and consumer found in the rules
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should ensure the user is authorized for each combination of ruleTypeId and consumer found in the rules
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should log an audit event when the rule fails at the authorization step
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should log an audit event when the rule fails at the authorization step
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should refresh the index of the event client once after scheduling the backfilling of all gaps
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should refresh the index of the event client once after scheduling the backfilling of all gaps
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should return a list with errored rules
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should return a list with errored rules
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should return a list with rules that succeeded
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should return a list with rules that succeeded
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should return a list with rules that were skipped
[job] [logs] Jest Tests #19 / bulkFillGapsByRuleIds should return a list with rules that were skipped
[job] [logs] FTR Configs #66 / discover/group6 discover unsaved changes badge should not show a badge after loading a saved search, only after changes

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`links`	100.6KB	100.6KB	+60.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`savedObjects`	10.5KB	10.5KB	+35.0B

History

💔 Build #328110 failed 532d4bd

cc @olapawlus

nreese

kibana-presentation changes LGTM
code review and tested in chrome

…anel (elastic#231168) Closes elastic#231164 When saving a Links panel to the library from the Dashboard, errors during the save process are not properly caught or displayed to the user. This PR fixes the error handling and ensures that an error toast is shown to the user. Video Demonstration of the fix: https://github.com/user-attachments/assets/a376975c-267b-401f-ad25-d493aef0c7e2

kibanamachine · 2025-08-15T11:49:21Z