Skip to content

Commit e86561d

Browse files
committed
feat(failure): surface recorded failures in a notification bell
Failures have been durable and actionable since #7269 and reportable from the editor since #7296, but the only way to see one was a dev-only list at the bottom of the processor's Documents view. This puts them in front of the person who can act. An action is declared as data, not rendered by rules the client hard-codes. A FailureKind offers each action with an audience (the file's owner, the team's reviewer, or anyone who can see the row) and a slot, and the server resolves both against who is reading. Ownership is derived per reader from the row's actor rather than stored. So a new failure kind ships as a registry entry plus copy, and needs no frontend change at all. Which actions the server runs and which the client runs are now distinct: FailureActionId carries an Execution facet, the registry only requires a bean for a server action, and dispatching a client action is refused. Retry, decrypt-and-retry, view file and view in processor are the client's; dismiss stays the server's. The client hides an action it cannot perform rather than showing it disabled, and surfaces the server's reason as the row's note instead. A greyed-out button that can never work reads as false hope, and a note says the same thing honestly. Notifications derive from failures on read rather than living in a table of their own: one source today, and a table would need a write path, retention and a per-user read model before it earned itself. Every id on the notification endpoints is prefixed, so the bell can never hand a raw failure id to a failure endpoint. An attended policy run now records which document it was about, which is what lets a repeat fold onto one incident instead of opening a new one per upload, lets deleting the file clear its failure, and lets the owner open the document from the row. The failures list in the processor stays gated behind import.meta.env.DEV, and the bell's "view in processor" action is gated the same way, so it cannot point at a section that is not mounted. Both lift together when failures get their own review screen. The read scope itself is unchanged here: #7477 decides who may see which row, and this commit only decides what each reader is offered about a row they can already see.
1 parent 2155fc3 commit e86561d

61 files changed

Lines changed: 5881 additions & 195 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionException.java

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package stirling.software.proprietary.failure;
22

3+
import org.springframework.http.HttpStatus;
4+
35
import lombok.Getter;
46

57
/**
6-
* Why an action could not be dispatched. Carries a {@link Reason} rather than an HTTP status, so
7-
* the service stays web-agnostic and the controller owns the mapping.
8+
* Why an action could not be dispatched. Thrown carrying a {@link Reason} rather than an HTTP
9+
* status, so the service stays web-agnostic and only a controller ever reaches for {@link
10+
* #statusOf}.
811
*/
912
@Getter
1013
public class FailureActionException extends RuntimeException {
@@ -23,14 +26,17 @@ public enum Reason {
2326

2427
/**
2528
* The action exists but this kind does not declare it, so an incoherent pairing (releasing
26-
* a document whose destination is what failed) cannot be dispatched even by hand.
27-
*
28-
* <p>Unreachable today: both kinds declare both actions, so no request can trip this guard
29-
* until a kind ships with a restricted action set. Declared now because the guard must
30-
* exist before that kind does, not after.
29+
* a document whose destination is what failed) cannot be dispatched even by hand. Reachable
30+
* as of the kinds that stopped offering {@link FailureActionId#ACKNOWLEDGE}.
3131
*/
3232
ACTION_NOT_DECLARED,
3333

34+
/**
35+
* The kind offers it, but the client is what runs it (see {@link
36+
* FailureActionId.Execution#CLIENT}), so it is refused rather than half-performed.
37+
*/
38+
ACTION_NOT_DISPATCHABLE,
39+
3440
/** The event is already closed, so no further transition is possible. */
3541
ALREADY_CLOSED
3642
}
@@ -46,4 +52,22 @@ public FailureActionException(Reason reason, String message, Throwable cause) {
4652
super(message, cause);
4753
this.reason = reason;
4854
}
55+
56+
/**
57+
* The HTTP status each refusal reason answers with. Lives with the reasons it maps, so the two
58+
* surfaces that dispatch actions, the failure queue and the notification bell, cannot drift
59+
* apart and a client's error handling does not depend on which one it called.
60+
*
61+
* <p>A closed row is a conflict rather than a bad request: the request was well-formed and
62+
* would have been valid a moment earlier. A missing row is a 404 whether it never existed,
63+
* belongs to another team or is a colleague's, so trying does not confirm which.
64+
*/
65+
public static HttpStatus statusOf(Reason reason) {
66+
return switch (reason) {
67+
case EVENT_NOT_FOUND -> HttpStatus.NOT_FOUND;
68+
case ACTION_NOT_RECOGNISED, ACTION_NOT_DECLARED, ACTION_NOT_DISPATCHABLE ->
69+
HttpStatus.BAD_REQUEST;
70+
case ALREADY_CLOSED -> HttpStatus.CONFLICT;
71+
};
72+
}
4973
}
Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,70 @@
11
package stirling.software.proprietary.failure;
22

3+
import lombok.Getter;
4+
35
/**
4-
* The actions a {@link FailureKind} may declare. Both are incident dispositions: they change how
5-
* the event is shown and touch nothing else, which is what makes them valid for every kind
6-
* including {@link FailureKind#UNKNOWN}, and why there is no {@code APPROVE} yet.
6+
* The actions a {@link FailureKind} may declare, and which side of the wire runs each one.
7+
*
8+
* <p>The dispositions are the server's: they change how the event is shown and touch nothing else,
9+
* which is what makes them valid for every kind including {@link FailureKind#UNKNOWN}. Everything
10+
* else is the client's, because the server holds an opaque id for the document and nothing more.
11+
*
12+
* <p>Client actions are still declared here rather than invented by each client, so the server
13+
* keeps deciding what a kind offers, in what order and under what label.
714
*/
15+
@Getter
816
public enum FailureActionId {
9-
ACKNOWLEDGE,
10-
DISMISS
17+
18+
/**
19+
* "Seen, and I own it." No kind offers this any more, but rows shipped before that are already
20+
* {@code ACKNOWLEDGED} and must stay readable and closable.
21+
*/
22+
ACKNOWLEDGE(Execution.SERVER, "Acknowledge"),
23+
24+
/** "No remediation will happen; close it." See {@link DismissAction}. */
25+
DISMISS(Execution.SERVER, "Dismiss"),
26+
27+
/** Run the failed operation again on the document the client still holds. */
28+
RETRY(Execution.CLIENT, "Retry"),
29+
30+
/** Ask the owner for the password, unlock the document in their client, then retry. */
31+
DECRYPT_AND_RETRY(Execution.CLIENT, "Decrypt and retry"),
32+
33+
/** Open the document this incident is about. Only its owner's client can resolve the id. */
34+
VIEW_FILE(Execution.CLIENT, "View file"),
35+
36+
/** Open the run behind it, for whoever reviews the team rather than owns the document. */
37+
VIEW_IN_PROCESSOR(Execution.CLIENT, "View in processor");
38+
39+
/**
40+
* Which side of the wire runs the action. The dispatch endpoint refuses a {@code CLIENT} id, so
41+
* "the client does this one" is enforced rather than merely documented.
42+
*/
43+
public enum Execution {
44+
45+
/**
46+
* Dispatchable, and {@link FailureActionRegistry} requires a {@link FailureAction} bean.
47+
*/
48+
SERVER,
49+
50+
/**
51+
* Declared and rendered, never dispatched: the server has neither the file nor the tool.
52+
*/
53+
CLIENT
54+
}
55+
56+
private final Execution execution;
57+
58+
/** English fallback, used when the client has no translation for the action's label key. */
59+
private final String defaultLabel;
60+
61+
FailureActionId(Execution execution, String defaultLabel) {
62+
this.execution = execution;
63+
this.defaultLabel = defaultLabel;
64+
}
65+
66+
/** Whether the server runs it itself, which is also whether it can be dispatched. */
67+
public boolean runsOnServer() {
68+
return execution == Execution.SERVER;
69+
}
1170
}

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionRegistry.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
* Resolves a {@link FailureActionId} to the bean that implements it. The startup check is the
1717
* point: because kinds declare action ids as data, one could name an action nobody implements,
1818
* which would otherwise show up as a button that 400s rather than as a failed boot.
19+
*
20+
* <p>Only {@link FailureActionId.Execution#SERVER} ids belong here. A client action has no bean by
21+
* design, and a bean for one is refused outright because dispatch could never reach it.
1922
*/
2023
@Slf4j
2124
@Service
@@ -25,6 +28,14 @@ public class FailureActionRegistry {
2528

2629
public FailureActionRegistry(List<FailureAction> actions) {
2730
for (FailureAction action : actions) {
31+
if (!action.id().runsOnServer()) {
32+
throw new IllegalStateException(
33+
"Action "
34+
+ action.id()
35+
+ " is run by the client, so "
36+
+ action.getClass().getName()
37+
+ " could never be dispatched");
38+
}
2839
FailureAction clash = byId.put(action.id(), action);
2940
if (clash != null) {
3041
throw new IllegalStateException(
@@ -39,8 +50,8 @@ public FailureActionRegistry(List<FailureAction> actions) {
3950
}
4051

4152
/**
42-
* Fail fast if any kind declares an action with no handler, naming every gap rather than the
43-
* first, so one boot tells you everything that is missing.
53+
* Fail fast if any kind declares a server action with no handler, naming every gap rather than
54+
* the first, so one boot tells you everything that is missing.
4455
*/
4556
@PostConstruct
4657
void verifyEveryDeclaredActionHasAHandler() {
@@ -49,6 +60,7 @@ void verifyEveryDeclaredActionHasAHandler() {
4960
.flatMap(
5061
kind ->
5162
kind.getActions().stream()
63+
.filter(FailureActionId::runsOnServer)
5264
.filter(action -> !byId.containsKey(action))
5365
.map(action -> kind.getId() + " -> " + action))
5466
.toList();
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package stirling.software.proprietary.failure;
2+
3+
/**
4+
* Where a kind would like one of its actions to sit: the thing that fixes it, a supporting action,
5+
* or one folded away in a menu.
6+
*
7+
* <p>Intent, not layout. The client does the final promotion, because only it knows whether the
8+
* document is still in its own file store, and a resolution it cannot run is worth less than a
9+
* secondary action it can. Declaration order in {@link FailureKind} breaks a tie within a slot.
10+
*/
11+
public enum FailureActionSlot {
12+
13+
/** The action that resolves the failure. At most one per kind is worth declaring here. */
14+
RESOLUTION,
15+
16+
/** Offered alongside the resolution, for a caller the resolution is not aimed at. */
17+
SECONDARY,
18+
19+
/** Available but folded away: correct, rarely the next thing anyone wants to press. */
20+
OVERFLOW
21+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package stirling.software.proprietary.failure;
2+
3+
/**
4+
* Who an offered action is for. The read scope has already decided the caller may see the incident;
5+
* this decides which of its actions are theirs to take.
6+
*
7+
* <p>The distinction is possession, not seniority: a reviewer clearing up after a colleague cannot
8+
* supply that colleague's password or reach a document only their browser holds.
9+
*/
10+
public enum FailureAudience {
11+
12+
/** The person whose work failed, whose own client still holds the document. */
13+
OWNER,
14+
15+
/** Anyone who triages the team's incidents, whoever hit them. */
16+
TEAM_REVIEWER,
17+
18+
/** Everyone the incident is shown to at all. */
19+
ANYONE_WHO_SEES
20+
}

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java

Lines changed: 91 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
package stirling.software.proprietary.failure;
22

3-
import static stirling.software.proprietary.failure.FailureActionId.ACKNOWLEDGE;
3+
import static stirling.software.proprietary.failure.FailureActionId.DECRYPT_AND_RETRY;
44
import static stirling.software.proprietary.failure.FailureActionId.DISMISS;
5+
import static stirling.software.proprietary.failure.FailureActionId.RETRY;
6+
import static stirling.software.proprietary.failure.FailureActionId.VIEW_FILE;
7+
import static stirling.software.proprietary.failure.FailureActionId.VIEW_IN_PROCESSOR;
8+
import static stirling.software.proprietary.failure.FailureActionSlot.OVERFLOW;
9+
import static stirling.software.proprietary.failure.FailureActionSlot.SECONDARY;
10+
import static stirling.software.proprietary.failure.FailureAudience.ANYONE_WHO_SEES;
11+
import static stirling.software.proprietary.failure.FailureAudience.OWNER;
12+
import static stirling.software.proprietary.failure.FailureAudience.TEAM_REVIEWER;
513

614
import java.util.Arrays;
715
import java.util.HashMap;
@@ -20,13 +28,14 @@
2028
* The registry of failure kinds, described as data: a stable id, i18n keys and an English fallback
2129
* like {@code ExceptionUtils.ErrorCode}, plus the facets a review surface needs.
2230
*
23-
* <p>Actions are declared here but implemented in {@link FailureAction} beans resolved by id, so a
24-
* new kind ships as a registry entry plus copy. Two members today: {@link #UNKNOWN} gives every
25-
* failed run a record, and kinds get promoted out of it as production shows what occurs.
31+
* <p>Actions are declared here but run elsewhere: a server action in a {@link FailureAction} bean
32+
* resolved by id, a client action in the browser that holds the document. Either way a new kind
33+
* ships as a registry entry plus copy. Two members today: {@link #UNKNOWN} gives every failed run a
34+
* record, and kinds get promoted out of it as production shows what occurs.
2635
*
27-
* <p>A kind offers an acknowledgement only where there is something to acknowledge <em>doing</em>.
28-
* With nothing to fix, "seen it" and "clear it" are the same decision, so the row offers only the
29-
* one that clears it.
36+
* <p>Each offer also says who it is for and where the kind wants it, because the same incident is
37+
* read by the person who hit it and by whoever reviews after them: only the owner can supply a
38+
* password, only a reviewer wants the run.
3039
*/
3140
@Getter
3241
public enum FailureKind {
@@ -37,8 +46,13 @@ public enum FailureKind {
3746
FailureScope.FILE,
3847
errorCodes("E004"),
3948
fallback("This document is password-protected, so the pipeline could not read it."),
40-
offer(ACKNOWLEDGE),
41-
offer(DISMISS, "dismissSkipFile")),
49+
// The password is the fix and only the owner has it, so everyone else is
50+
// offered the run and a way to close the row.
51+
resolution(DECRYPT_AND_RETRY, OWNER),
52+
global(RETRY, OWNER, OVERFLOW),
53+
global(VIEW_FILE, OWNER, OVERFLOW),
54+
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, SECONDARY),
55+
global(DISMISS, ANYONE_WHO_SEES, OVERFLOW)),
4256

4357
UNKNOWN(
4458
FailureStage.INTERNAL,
@@ -47,7 +61,12 @@ public enum FailureKind {
4761
FailureScope.RUN,
4862
noErrorCodes(),
4963
fallback("This run failed for a reason Stirling does not yet recognise."),
50-
offer(DISMISS));
64+
// Nothing here is known to be fixable, so there is no resolution to declare. A plain
65+
// retry is still worth offering: an unrecognised failure is often a one-off.
66+
global(RETRY, OWNER, SECONDARY),
67+
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, SECONDARY),
68+
global(VIEW_FILE, OWNER, OVERFLOW),
69+
global(DISMISS, ANYONE_WHO_SEES, OVERFLOW));
5170

5271
private static final String KEY_PREFIX = "portal.failures.kind.";
5372
private static final String ACTION_KEY_PREFIX = "portal.failures.action.";
@@ -95,22 +114,52 @@ public enum FailureKind {
95114
}
96115

97116
/**
98-
* One action this kind offers, with the key to label it by. One ordered list rather than ids
99-
* plus a parallel map of overrides, which could disagree with each other.
117+
* One action this kind offers: who it is for, where it wants to sit, and the key to label it
118+
* by. One ordered list rather than ids plus parallel maps of audiences, slots and label
119+
* overrides, which could disagree with each other.
100120
*
101121
* @param labelKeySuffix key under {@code portal.failures.action.}, or null for the generic
102122
* label
103123
*/
104-
private record Offer(FailureActionId id, String labelKeySuffix) {}
124+
private record Offer(
125+
FailureActionId id,
126+
FailureAudience audience,
127+
FailureActionSlot slot,
128+
String labelKeySuffix) {}
105129

106-
/** An action labelled by this kind's own wording, where the generic label reads badly. */
107-
private static Offer offer(FailureActionId id, String labelKeySuffix) {
108-
return new Offer(id, labelKeySuffix);
130+
/**
131+
* The action that fixes this kind, for whoever can actually apply it. In the resolution slot by
132+
* definition: a kind needing two of these would be two kinds.
133+
*/
134+
private static Offer resolution(FailureActionId id, FailureAudience audience) {
135+
return new Offer(id, audience, FailureActionSlot.RESOLUTION, null);
136+
}
137+
138+
/** As {@link #resolution(FailureActionId, FailureAudience)}, with this kind's own wording. */
139+
private static Offer resolution(
140+
FailureActionId id, FailureAudience audience, String labelKeySuffix) {
141+
return new Offer(id, audience, FailureActionSlot.RESOLUTION, labelKeySuffix);
142+
}
143+
144+
/**
145+
* An action that is not this kind's fix: the same offer any kind can make, placed where this
146+
* kind wants it and labelled by the shared wording.
147+
*/
148+
private static Offer global(
149+
FailureActionId id, FailureAudience audience, FailureActionSlot slot) {
150+
return new Offer(id, audience, slot, null);
109151
}
110152

111-
/** An action labelled by the shared wording for that action. */
112-
private static Offer offer(FailureActionId id) {
113-
return new Offer(id, null);
153+
/**
154+
* As {@link #global(FailureActionId, FailureAudience, FailureActionSlot)}, but labelled by this
155+
* kind's own wording where the shared one reads badly.
156+
*/
157+
private static Offer global(
158+
FailureActionId id,
159+
FailureAudience audience,
160+
FailureActionSlot slot,
161+
String labelKeySuffix) {
162+
return new Offer(id, audience, slot, labelKeySuffix);
114163
}
115164

116165
/**
@@ -149,6 +198,29 @@ public List<FailureActionId> getActions() {
149198
return offers.stream().map(Offer::id).toList();
150199
}
151200

201+
/**
202+
* What this kind offers, in declaration order, each with its label and placement resolved. What
203+
* a review surface reads, so it never has to ask three separate questions about one offer.
204+
*/
205+
public List<OfferedAction> getOfferedActions() {
206+
return offers.stream()
207+
.map(
208+
offer ->
209+
new OfferedAction(
210+
offer.id(),
211+
labelKeyFor(offer.id()),
212+
offer.audience(),
213+
offer.slot()))
214+
.toList();
215+
}
216+
217+
/** One action as a kind declares it: what to call it, who it is for, where it wants to sit. */
218+
public record OfferedAction(
219+
FailureActionId id,
220+
String labelKey,
221+
FailureAudience audience,
222+
FailureActionSlot slot) {}
223+
152224
/** Whether this kind offers {@code action}. The dispatch guard: see {@code FailureActionId}. */
153225
public boolean declares(FailureActionId action) {
154226
return offers.stream().anyMatch(offer -> offer.id() == action);

0 commit comments

Comments
 (0)