Skip to content

Commit

Permalink
fw lite handle errors on frontend (#947)
Browse files Browse the repository at this point in the history
* dynamically load project when trying to access LcmCache in FwDataMiniLcmApi

* notify client when a project is locked, display a message and allow the user to retry on frontend.

* setup generic signalr error handling

* hide double notifications for locked projects, change the error message to something more helpful

* set sense part of speech id to undefined by default

* fix bug where a new id would be generated even if it was already set for both the sense and example sentence

* fix bug where reloading the page would result in a 404, fixed by setting up a fallback to the index page

* add placeholder for index changes
  • Loading branch information
hahn-kev authored Jul 12, 2024
1 parent fd0eba8 commit 614c0b6
Show file tree
Hide file tree
Showing 18 changed files with 261 additions and 121 deletions.
140 changes: 62 additions & 78 deletions backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions backend/FwLite/FwDataMiniLcmBridge/FwDataFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ public FwDataMiniLcmApi GetFwDataMiniLcmApi(string projectName, bool saveOnDispo

public FwDataMiniLcmApi GetFwDataMiniLcmApi(FwDataProject project, bool saveOnDispose)
{
var projectService = GetProjectServiceCached(project);
return new FwDataMiniLcmApi(projectService, saveOnDispose, fwdataLogger, project);
return new FwDataMiniLcmApi(new (() =>GetProjectServiceCached(project)), saveOnDispose, fwdataLogger, project);
}

private HashSet<string> _projects = [];
Expand Down
8 changes: 1 addition & 7 deletions backend/FwLite/LocalWebApp/Hubs/CrdtMiniLcmApiHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,11 @@

namespace LocalWebApp.Hubs;

public interface ILexboxClient
{
Task OnEntryUpdated(Entry entry);
Task OnProjectClosed();
}

public class CrdtMiniLcmApiHub(
ILexboxApi lexboxApi,
IOptions<JsonOptions> jsonOptions,

Check warning on line 11 in backend/FwLite/LocalWebApp/Hubs/CrdtMiniLcmApiHub.cs

View workflow job for this annotation

GitHub Actions / Build FW Lite

Parameter 'jsonOptions' is unread.

Check warning on line 11 in backend/FwLite/LocalWebApp/Hubs/CrdtMiniLcmApiHub.cs

View workflow job for this annotation

GitHub Actions / Build FW Lite

Parameter 'jsonOptions' is unread.

Check warning on line 11 in backend/FwLite/LocalWebApp/Hubs/CrdtMiniLcmApiHub.cs

View workflow job for this annotation

GitHub Actions / Build FW Lite

Parameter 'jsonOptions' is unread.

Check warning on line 11 in backend/FwLite/LocalWebApp/Hubs/CrdtMiniLcmApiHub.cs

View workflow job for this annotation

GitHub Actions / Build FW Lite

Parameter 'jsonOptions' is unread.
BackgroundSyncService backgroundSyncService,
SyncService syncService) : Hub<ILexboxClient>
SyncService syncService) : Hub<ILexboxHubClient>
{
public const string ProjectRouteKey = "project";
public override async Task OnConnectedAsync()
Expand Down
13 changes: 11 additions & 2 deletions backend/FwLite/LocalWebApp/Hubs/FwDataMiniLcmHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
using Microsoft.AspNetCore.SignalR;
using Microsoft.Extensions.Options;
using MiniLcm;
using SIL.LCModel;
using SystemTextJsonPatch;

namespace LocalWebApp.Hubs;

public class FwDataMiniLcmHub([FromKeyedServices(FwDataBridgeKernel.FwDataApiKey)] ILexboxApi lexboxApi, FwDataFactory fwDataFactory,
FwDataProjectContext context) : Hub<ILexboxClient>
FwDataProjectContext context) : Hub<ILexboxHubClient>
{
public const string ProjectRouteKey = "fwdata";
public override async Task OnConnectedAsync()
Expand All @@ -29,7 +30,15 @@ public override async Task OnDisconnectedAsync(Exception? exception)
{
throw new InvalidOperationException("No project is set in the context.");
}
await Clients.OthersInGroup(project.Name).OnProjectClosed();

if (exception is LcmFileLockedException)
{
await Clients.Group(project.Name).OnProjectClosed(CloseReason.Locked);
}
else
{
await Clients.OthersInGroup(project.Name).OnProjectClosed(CloseReason.User);
}
await Groups.RemoveFromGroupAsync(Context.ConnectionId, project.Name);
}

Expand Down
15 changes: 15 additions & 0 deletions backend/FwLite/LocalWebApp/Hubs/ILexboxHubClient.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using MiniLcm;

namespace LocalWebApp.Hubs;

public interface ILexboxHubClient
{
Task OnEntryUpdated(Entry entry);
Task OnProjectClosed(CloseReason reason);
}

public enum CloseReason
{
User,
Locked
}
34 changes: 34 additions & 0 deletions backend/FwLite/LocalWebApp/Hubs/LockedProjectFilter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using Microsoft.AspNetCore.SignalR;
using SIL.LCModel;

namespace LocalWebApp.Hubs;

public class LockedProjectFilter: IHubFilter
{
public async ValueTask<object?> InvokeMethodAsync(HubInvocationContext invocationContext, Func<HubInvocationContext, ValueTask<object?>> next)
{
try
{
return await next(invocationContext);
}
catch (LcmFileLockedException)
{
await TypedHubHelper<ILexboxHubClient>.TypeClients(invocationContext.Hub.Clients)
.Caller.OnProjectClosed(CloseReason.Locked);
throw new HubException("The project is locked.");
}
}

private class TypedHubHelper<TClient> : Hub<TClient> where TClient : class
{
public TypedHubHelper(IHubCallerClients clients) : base()
{
((Hub)this).Clients = clients;
}

public static IHubCallerClients<TClient> TypeClients(IHubCallerClients clients)
{
return new TypedHubHelper<TClient>(clients).Clients;
}
}
}
11 changes: 9 additions & 2 deletions backend/FwLite/LocalWebApp/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using LocalWebApp.Auth;
using LocalWebApp.Routes;
using LocalWebApp.Utils;
using Microsoft.AspNetCore.SignalR;
using Microsoft.AspNetCore.StaticFiles.Infrastructure;
using Microsoft.Extensions.FileProviders;

Expand All @@ -25,7 +26,11 @@
builder.Services.AddLocalAppServices(builder.Environment);
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();
builder.Services.AddSignalR().AddJsonProtocol();
builder.Services.AddSignalR(options =>
{
options.AddFilter(new LockedProjectFilter());
options.EnableDetailedErrors = true;
}).AddJsonProtocol();

var app = builder.Build();
// Configure the HTTP request pipeline.
Expand All @@ -38,7 +43,8 @@
//configure dotnet to serve static files from the embedded resources
var sharedOptions = new SharedOptions() { FileProvider = new ManifestEmbeddedFileProvider(typeof(Program).Assembly) };
app.UseDefaultFiles(new DefaultFilesOptions(sharedOptions));
app.UseStaticFiles(new StaticFileOptions(sharedOptions));
var staticFileOptions = new StaticFileOptions(sharedOptions);
app.UseStaticFiles(staticFileOptions);

app.Use(async (context, next) =>
{
Expand Down Expand Up @@ -68,6 +74,7 @@
app.MapTest();
app.MapImport();
app.MapAuthRoutes();
app.MapFallbackToFile("index.html", staticFileOptions);

await using (app)
{
Expand Down
62 changes: 51 additions & 11 deletions frontend/viewer/src/FwDataProjectView.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,65 @@
import ProjectView from './ProjectView.svelte';
import {navigate} from 'svelte-routing';
import {AppNotification} from './lib/notifications/notifications';
import {CloseReason} from './lib/generated-signalr-client/TypedSignalR.Client/Lexbox.ClientServer.Hubs';
import {Entry} from './lib/mini-lcm';
export let projectName: string;
setContext('project-name', projectName);
const connection = new HubConnectionBuilder()
.withUrl(`/api/hub/${projectName}/fwdata`)
.withAutomaticReconnect()
.build();
void connection.start()
.then(() => connected = (connection.state == HubConnectionState.Connected))
.catch(err => console.error(err));
function connect() {
void connection.start()
.then(() => connected = (connection.state == HubConnectionState.Connected))
.catch(err => {
console.error('Failed to start the connection:', err);
});
}
connect();
onDestroy(() => connection.stop());
setContext('project-name', projectName);
SetupSignalR(connection, {
history: false,
write: true,
},
async () => {
navigate('/');
AppNotification.display('Project closed on another tab', 'warning', 'long');
connection.onclose(error => {
connected = false;
if (!error) return;
console.error('Connection closed:', error);
});
SetupSignalR(connection, {
history: false,
write: true,
},
{
OnEntryUpdated: async (entry: Entry) => {
console.log('OnEntryUpdated', entry);
},
async OnProjectClosed(reason: CloseReason): Promise<void> {
connected = false;
switch (reason) {
case CloseReason.User:
navigate('/');
AppNotification.display('Project closed on another tab', 'warning', 'long');
break;
case CloseReason.Locked:
AppNotification.displayAction('The project is open in FieldWorks. Please close it and try again.', 'warning', {
label: 'Retry',
callback: () => connected = true
});
break;
}
}
},
(errorContext) => {
connected = false;
if (errorContext.error instanceof Error) {
let message = errorContext.error.message;
if (message.includes('The project is locked')) return; //handled via the project closed callback
AppNotification.display('Connection error: ' + message, 'error', 'long');
} else {
AppNotification.display('Unknown Connection error', 'error', 'long');
}
}
);
let connected = false;
</script>
<ProjectView {projectName} isConnected={connected}></ProjectView>
22 changes: 22 additions & 0 deletions frontend/viewer/src/lib/Editor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@
await updateEntry(e.entry);
if (e.sense !== undefined) {
await updateSense(e.sense);
detectSenseIndexChanges(e.entry, e.sense);
if (e.example !== undefined) {
await updateExample(e.sense.id, e.example);
detectExampleIndexChanges(e.entry, e.sense, e.example);
}
}
Expand Down Expand Up @@ -95,6 +97,26 @@
if (operations.length == 0) return;
await saveHandler(() => lexboxApi.UpdateExampleSentence(entry.id, senseId, updatedExample.id, operations));
}
function detectSenseIndexChanges(entry: IEntry, sense: ISense) {
const initialIndex = initialEntry.senses.findIndex(s => s.id === sense.id);
if (initialIndex === -1) return;
const currentIndex = entry.senses.findIndex(s => s.id === sense.id);
if (currentIndex === -1) return;
if (initialIndex !== currentIndex) {
// todo figure out how to send this to the server
}
}
function detectExampleIndexChanges(entry: IEntry, sense: ISense, example: IExampleSentence) {
const initialIndex = initialEntry.senses.find(s => s.id == sense.id)?.exampleSentences.findIndex(s => s.id === example.id);
if (initialIndex === -1 || initialIndex === undefined) return;
const currentIndex = sense.exampleSentences.findIndex(s => s.id === example.id);
if (currentIndex === -1) return;
if (initialIndex !== currentIndex) {
// todo figure out how to send this to the server
}
}
</script>

<div id="entry" class:hide-empty={$viewConfig.hideEmptyFields}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import CrdtField from './CrdtField.svelte';
import { SelectField, TextField, type MenuOption } from 'svelte-ux';
export let value: string;
export let value: string | undefined;
export let unsavedChanges = false;
export let options: MenuOption[] | undefined = undefined;
export let label: string | undefined = undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
type T = $$Generic<{}>;
export let field: FieldConfig & OptionFieldConfig;
export let value: string;
export let value: string | undefined;
let options: Readable<MenuOption[]> = readable([]);
$: options = pickOptions(field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ export type ILexboxClient = {
* @returns Transpiled from System.Threading.Tasks.Task
*/
OnEntryUpdated(entry: Entry): Promise<void>;
OnProjectClosed(): Promise<void>;
OnProjectClosed(reason: CloseReason): Promise<void>;
}

export enum CloseReason {
User = 0,
Locked = 1,
}
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ class ILexboxClient_Binder implements ReceiverRegister<ILexboxClient> {

public readonly register = (connection: HubConnection, receiver: ILexboxClient): Disposable => {

const __onEntryUpdated = (...args: [Entry]) => receiver.OnEntryUpdated(...args);
const __onProjectClosed = () => receiver.OnProjectClosed();
const __onEntryUpdated = (...args: Parameters<ILexboxClient['OnEntryUpdated']>) => receiver.OnEntryUpdated(...args);
const __onProjectClosed = (...args: Parameters<ILexboxClient['OnProjectClosed']>) => receiver.OnProjectClosed(...args);

connection.on("OnEntryUpdated", __onEntryUpdated);
connection.on("OnProjectClosed", __onProjectClosed);
Expand Down
2 changes: 1 addition & 1 deletion frontend/viewer/src/lib/mini-lcm/i-sense.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export interface ISense {
id: string;
definition: IMultiString;
gloss: IMultiString;
partOfSpeechId: string;
partOfSpeechId: string | undefined;
semanticDomains: SemanticDomain[];
exampleSentences: IExampleSentence[];
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script lang="ts">
import {AppNotification} from './notifications';
import {Notification, Icon} from 'svelte-ux';
import {Notification, Icon, Button} from 'svelte-ux';
import {
mdiAlert,
mdiAlertCircleOutline,
Expand All @@ -13,7 +13,7 @@
<div class="fixed bottom-0 z-50 flex flex-col gap-2 p-4 w-full overflow-y-auto">
{#each $notifications as notification}
<div class="w-[400px] mx-auto">
<Notification open closeIcon>
<Notification open closeIcon actions="right">
<div slot="icon">
{#if notification.type === 'success'}
<Icon path={mdiCheckCircleOutline} size="1.5rem" class="text-success"/>
Expand All @@ -26,6 +26,11 @@
{/if}
</div>
<div slot="title">{notification.message}</div>
<div slot="actions">
{#if notification.action}
<Button color="primary" on:click={notification.action.callback}>{notification.action.label}</Button>
{/if}
</div>
</Notification>
</div>
{/each}
Expand Down
14 changes: 13 additions & 1 deletion frontend/viewer/src/lib/notifications/notifications.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import {writable, type Writable, type Readable, readonly} from 'svelte/store';

interface NotificationAction {
label: string;
callback: () => void;
}

export class AppNotification {
private static _notifications: Writable<AppNotification[]> = writable([]);
public static get notifications(): Writable<AppNotification[]> {
return this._notifications;
}

public static display(message: string, type: 'success' | 'error' | 'info' | 'warning', timeout: 'short' | 'long' | number = 'short') {
const notification = new AppNotification(message, type);
this._notifications.update(notifications => [...notifications, notification]);
Expand All @@ -17,5 +23,11 @@ export class AppNotification {
}, timeout);
}

private constructor(public message: string, public type: 'success' | 'error' | 'info' | 'warning') {}
public static displayAction(message: string, type: 'success' | 'error' | 'info' | 'warning', action: NotificationAction) {
const notification = new AppNotification(message, type, action);
this._notifications.update(notifications => [...notifications, notification]);
}

private constructor(public message: string, public type: 'success' | 'error' | 'info' | 'warning', public action?: NotificationAction) {
}
}
Loading

0 comments on commit 614c0b6

Please sign in to comment.