Skip to content

Commit d4612e3

Browse files
authored
enhance: remove auto delete and see logs of mcp failure on setup (#4611)
* enhance: change auto-delete on failed connect of user mcp servers & show deployment logs update * styling chnages * remove console.log * fix: allow logs through if server is running but not healthy
1 parent 207ac20 commit d4612e3

File tree

4 files changed

+154
-26
lines changed

4 files changed

+154
-26
lines changed

pkg/mcp/details.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package mcp
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"io"
78

@@ -24,6 +25,18 @@ func (sm *SessionManager) GetServerDetails(ctx context.Context, userID, mcpServe
2425
return sm.backend.getServerDetails(ctx, serverConfig.Scope)
2526
}
2627

28+
// shouldAttemptLogStreaming checks if the error is one that should still have logs
29+
// These are errors where the server might be running but not healthy
30+
func shouldAttemptLogStreaming(err error) bool {
31+
for unwrappedErr := err; unwrappedErr != nil; unwrappedErr = errors.Unwrap(unwrappedErr) {
32+
switch unwrappedErr {
33+
case ErrHealthCheckFailed, ErrHealthCheckTimeout, ErrPodConfigurationFailed:
34+
return true
35+
}
36+
}
37+
return false
38+
}
39+
2740
// StreamServerLogs will stream the logs of a specific MCP server based on its configuration, if the backend supports it.
2841
// If the server is remote, it will return an error as remote servers do not support this operation.
2942
// If the backend does not support the operation, it will return an [ErrNotSupportedByBackend] error.
@@ -34,7 +47,10 @@ func (sm *SessionManager) StreamServerLogs(ctx context.Context, userID, mcpServe
3447

3548
_, err := sm.ensureDeployment(ctx, serverConfig, userID, mcpServerDisplayName, mcpServerName)
3649
if err != nil {
37-
return nil, err
50+
// If error of deployment is not one that should still have logs, return the error
51+
if !shouldAttemptLogStreaming(err) {
52+
return nil, err
53+
}
3854
}
3955

4056
return sm.backend.streamServerLogs(ctx, serverConfig.Scope)

ui/user/src/lib/components/PageLoading.svelte

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
error?: string;
1717
errorPreContent?: Snippet;
1818
errorPostContent?: Snippet;
19+
errorClasses?: {
20+
root?: string;
21+
};
1922
onClose?: () => void;
2023
}
2124
@@ -27,6 +30,7 @@
2730
progress,
2831
isProgressBar,
2932
error,
33+
errorClasses,
3034
errorPreContent,
3135
errorPostContent,
3236
onClose
@@ -80,7 +84,10 @@
8084
>
8185
{#if error}
8286
<div
83-
class="dark:bg-surface2 dark:border-surface3 relative flex w-full flex-col items-center gap-4 rounded-lg bg-white p-4 md:w-md dark:border"
87+
class={twMerge(
88+
'dark:bg-surface2 dark:border-surface3 relative flex w-full flex-col items-center gap-4 rounded-lg bg-white p-4 dark:border',
89+
errorClasses?.root
90+
)}
8491
use:clickOutside={() => onClose?.()}
8592
>
8693
<button class="icon-button absolute top-2 right-2 self-end" onclick={() => onClose?.()}
@@ -106,8 +113,6 @@
106113
{#if errorPostContent}
107114
{@render errorPostContent()}
108115
{/if}
109-
110-
<button class="button w-full" onclick={() => onClose?.()}>Close</button>
111116
</div>
112117
{:else if isProgressBar}
113118
<div

ui/user/src/lib/components/mcp/MyMcpServers.svelte

Lines changed: 122 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import { twMerge } from 'tailwind-merge';
2626
import McpServerInfoAndTools from './McpServerInfoAndTools.svelte';
2727
import PageLoading from '../PageLoading.svelte';
28+
import { EventStreamService } from '$lib/services/admin/eventstream.svelte';
2829
2930
type Entry = MCPCatalogEntry & {
3031
categories: string[]; // categories for the entry
@@ -101,6 +102,9 @@
101102
let launching = $state(false);
102103
let launchError = $state<string>();
103104
let launchProgress = $state<number>(0);
105+
let launchLogsEventStream = $state<EventStreamService<string>>();
106+
let launchLogs = $state<string[]>([]);
107+
let relaunching = $state(false);
104108
105109
let deletingInstance = $state<MCPServerInstance>();
106110
let deletingServer = $state<MCPCatalogServer>();
@@ -250,12 +254,28 @@
250254
deletingServer = undefined;
251255
}
252256
253-
async function handleLaunchCatalogEntry(entry: Entry) {
257+
function listLaunchLogs(mcpServerId: string) {
258+
launchLogsEventStream = new EventStreamService<string>();
259+
launchLogsEventStream.connect(`/api/mcp-servers/${mcpServerId}/logs`, {
260+
onMessage: (data) => {
261+
launchLogs = [...launchLogs, data];
262+
}
263+
});
264+
}
265+
266+
async function handleLaunchCatalogEntry(entry: Entry, retryingServer?: MCPCatalogServer) {
254267
if (!entry.manifest) {
255268
console.error('No server manifest found');
256269
return;
257270
}
258271
272+
if (launchLogsEventStream) {
273+
// reset launch logs
274+
launchLogsEventStream.disconnect();
275+
launchLogsEventStream = undefined;
276+
launchLogs = [];
277+
}
278+
259279
launchError = undefined;
260280
launchProgress = 0;
261281
launching = true;
@@ -279,14 +299,18 @@
279299
const aliasToUse = configureForm?.name || getUniqueAlias(serverName);
280300
281301
let response: MCPCatalogServer | undefined = undefined;
282-
try {
283-
response = await ChatService.createSingleOrRemoteMcpServer({
284-
catalogEntryID: entry.id,
285-
manifest: url ? { remoteConfig: { url } } : {},
286-
alias: aliasToUse
287-
});
288-
} catch (err) {
289-
launchError = err instanceof Error ? err.message : 'An unknown error occurred';
302+
if (!retryingServer) {
303+
try {
304+
response = await ChatService.createSingleOrRemoteMcpServer({
305+
catalogEntryID: entry.id,
306+
manifest: url ? { remoteConfig: { url } } : {},
307+
alias: aliasToUse
308+
});
309+
} catch (err) {
310+
launchError = err instanceof Error ? err.message : 'An unknown error occurred';
311+
}
312+
} else {
313+
response = retryingServer;
290314
}
291315
292316
if (response) {
@@ -301,19 +325,20 @@
301325
configuredResponse.id
302326
);
303327
if (!launchResponse.success) {
304-
// because something failed, go ahead and delete the server we created
305328
launchError = launchResponse.message;
306-
await ChatService.deleteSingleOrRemoteMcpServer(configuredResponse.id);
329+
listLaunchLogs(configuredResponse.id);
307330
} else {
308331
launchProgress = 100;
332+
}
309333
310-
selectedEntryOrServer = {
311-
server: configuredResponse,
312-
connectURL: configuredResponse.connectURL,
313-
instance: undefined,
314-
parent: entry
315-
} as ConnectedServer;
334+
selectedEntryOrServer = {
335+
server: configuredResponse,
336+
connectURL: configuredResponse.connectURL,
337+
instance: undefined,
338+
parent: entry
339+
} as ConnectedServer;
316340
341+
if (!launchError) {
317342
const ref = selectedEntryOrServer;
318343
setTimeout(() => {
319344
launching = false;
@@ -322,12 +347,13 @@
322347
}, 1000);
323348
}
324349
} catch (err) {
325-
await ChatService.deleteSingleOrRemoteMcpServer(response.id);
326350
launchError = err instanceof Error ? err.message : 'An unknown error occurred';
327351
} finally {
328352
clearTimeout(timeout1);
329353
clearTimeout(timeout2);
330354
clearTimeout(timeout3);
355+
356+
relaunching = false;
331357
}
332358
}
333359
}
@@ -401,6 +427,18 @@
401427
if (!selectedEntryOrServer) return;
402428
if (!configureForm) return;
403429
430+
if (
431+
relaunching &&
432+
'parent' in selectedEntryOrServer &&
433+
'server' in selectedEntryOrServer &&
434+
selectedEntryOrServer.parent &&
435+
selectedEntryOrServer.server
436+
) {
437+
configDialog?.close();
438+
await handleLaunchCatalogEntry(selectedEntryOrServer.parent, selectedEntryOrServer.server);
439+
return;
440+
}
441+
404442
try {
405443
if ('server' in selectedEntryOrServer && selectedEntryOrServer.server?.id) {
406444
if (
@@ -484,6 +522,23 @@
484522
configDialog?.open();
485523
}
486524
525+
async function handleCancelLaunch() {
526+
if (launchLogsEventStream) {
527+
launchLogsEventStream.disconnect();
528+
}
529+
if (
530+
selectedEntryOrServer &&
531+
'server' in selectedEntryOrServer &&
532+
selectedEntryOrServer.server
533+
) {
534+
await ChatService.deleteSingleOrRemoteMcpServer(selectedEntryOrServer.server.id);
535+
selectedEntryOrServer = selectedEntryOrServer.parent;
536+
}
537+
538+
launching = false;
539+
launchError = undefined;
540+
}
541+
487542
const duration = PAGE_TRANSITION_DURATION;
488543
</script>
489544

@@ -654,17 +709,62 @@
654709
text="Configuring and initializing server..."
655710
progress={launchProgress}
656711
error={launchError}
657-
onClose={() => {
658-
launching = false;
712+
errorClasses={{
713+
root: 'md:w-[95vw]'
659714
}}
715+
onClose={handleCancelLaunch}
660716
>
661717
{#snippet errorPreContent()}
662718
<h4 class="text-xl font-semibold">MCP Server Launch Failed</h4>
663719
{/snippet}
664720
{#snippet errorPostContent()}
665-
<p class="text-md self-start">An issue occurred while launching the MCP server.</p>
721+
{@const hasConfigurableParent =
722+
selectedEntryOrServer &&
723+
'parent' in selectedEntryOrServer &&
724+
selectedEntryOrServer.parent &&
725+
hasEditableConfiguration(selectedEntryOrServer.parent)}
726+
{#if launchLogs.length > 0}
727+
<div
728+
class="default-scrollbar-thin bg-surface1 max-h-[50vh] w-full overflow-y-auto rounded-lg p-4 shadow-inner"
729+
>
730+
{#each launchLogs as log, i (i)}
731+
<div class="font-mono text-sm">
732+
<span class="text-gray-600 dark:text-gray-400">{log}</span>
733+
</div>
734+
{/each}
735+
</div>
736+
{:else}
737+
<p class="text-md self-start">An issue occurred while launching the MCP server.</p>
738+
{/if}
666739

667-
<p class="text-md self-start">If the problem persists, please contact an administrator.</p>
740+
<div class="flex w-full flex-col items-center gap-2 md:flex-row">
741+
{#if hasConfigurableParent}
742+
<button
743+
class="button-primary w-full md:w-1/2 md:flex-1"
744+
onclick={() => {
745+
launching = false;
746+
launchError = undefined;
747+
relaunching = true;
748+
if (
749+
selectedEntryOrServer &&
750+
'parent' in selectedEntryOrServer &&
751+
selectedEntryOrServer.parent
752+
) {
753+
if (hasEditableConfiguration(selectedEntryOrServer.parent)) {
754+
configDialog?.open();
755+
} else {
756+
handleLaunch();
757+
}
758+
}
759+
}}
760+
>
761+
Update Configuration and Try Again
762+
</button>
763+
{/if}
764+
<button class="button w-full md:w-1/2 md:flex-1" onclick={handleCancelLaunch}>
765+
Cancel and Delete Server
766+
</button>
767+
</div>
668768
{/snippet}
669769
</PageLoading>
670770

ui/user/src/lib/services/chat/operations.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1636,6 +1636,13 @@ export async function validateSingleOrRemoteMcpServerLaunched(mcpServerId: strin
16361636
}
16371637
}
16381638

1639+
export async function listSingleOrRemoteMcpServerLogs(mcpServerId: string): Promise<string[]> {
1640+
const response = (await doGet(`/mcp-servers/${mcpServerId}/logs`, {
1641+
dontLogErrors: true
1642+
})) as ItemsResponse<string>;
1643+
return response.items ?? [];
1644+
}
1645+
16391646
export async function listWorkspaces(opts?: { fetch?: Fetcher }): Promise<Workspace[]> {
16401647
const response = (await doGet('/workspaces', opts)) as ItemsResponse<Workspace>;
16411648
return response.items ?? [];

0 commit comments

Comments
 (0)