Handle rejected sortable request instead of leaving it unhandled

[dg]

Sortable ignores the promise returned from onEnd, so when the sort request failed (network error, or a handler responding with an empty body where Naja expects JSON), the rejection surfaced as an uncaught promise error in the console with no context. The request is now settled in makeSortRequest itself and a failure is logged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.12%. Comparing base (ab5506d) to head (fbefc16).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1301   +/-   ##
=======================================
  Coverage   49.12%   49.12%           
=======================================
  Files          63       63           
  Lines        2974     2974           
=======================================
  Hits         1461     1461           
  Misses       1513     1513           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR addresses unhandled promise rejections triggered by failed SortableJS sort requests by ensuring the underlying AJAX request is always settled within makeSortRequest, and logging failures for visibility.

Changes:

• Wraps datagrid.ajax.request(...) with handlers so its rejection is consumed even when SortableJS ignores the returned promise.
• Logs an error when the sort request fails to avoid silent/uncontextualized failures in the console.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[radimvaculik]

Thanks — the unhandled rejection is a real problem. SortableJS drops the return value of onEnd, so a failed request ends up as a bare Uncaught (in promise) with no context. Agreed it needs handling. CI is green and the build is fine, so this is only about which shape we want.

Two things I'd like to think through before merging.

1. console.error in library code

NajaAjax.onInit() already re-dispatches Naja's error event on datagrid.ajax, so applications can already observe the failure. This adds a second, non-configurable output that consumers can't turn off.

If we keep the logging approach, .catch() is equivalent to the two-argument .then() here and reads a bit simpler:

return datagrid.ajax.request({ method: "GET", url, data })
	.catch((error: unknown) => console.error("Datagrid: sortable request failed", { url, data, error }));

Alternatively, propagate it outwards and let the app decide (flash message, toast, reload):

return datagrid.ajax.request({ method: "GET", url, data })
	.catch((error: unknown) => {
		datagrid.ajax.dispatch("sortError", { item, params: { url, data }, error });
	});

2. The grid keeps lying after a failure

This is the part that bothers me more. When the request fails, the row stays visually in its new position while the server order is unchanged — the user sees a sorted table that isn't sorted, and the only trace is a console message they'll never look at.

plugins/features/editable.ts handles the analogous case by reverting the cell value in its catch. SortableJS gives us everything needed to do the same for a drag, since onEnd exposes from and oldIndex:

onEnd: ({ item, from, oldIndex }) => {
	return this.makeSortRequest(datagrid, item, datagrid.el.querySelector("tbody"))
		?.catch((error: unknown) => {
			from.insertBefore(item, from.children[oldIndex!] ?? null);
			datagrid.ajax.dispatch("sortError", { item, error });
		});
},

(Sketch, not tested — initSortableTree needs the same treatment, and cross-list drags there must go back to from, not to the current parent.)

That way the DOM stays in sync with the server, and an app that wants to show a message can still listen for the event.

---

Happy to merge this as-is if you'd rather keep the scope minimal — an uncaught rejection is worse than a log, and the revert can land separately. Just wanted to check whether you'd be up for the revert-and-dispatch variant instead, since it fixes the actual user-visible symptom rather than the console noise.