mirror of
https://github.com/block/buzz.git
synced 2026-11-14 05:12:27 +00:00
fix(mobile): skip an archived starter at desktop's id
Desktop and mobile derive the same starter channel id, so an archived starter left at that id makes the relay answer "duplicate: channel already exists". The directory omits archived channels, so the reload found no holder and the invite stayed stuck in setup. When a create is refused as a duplicate and the reload has no match, read the channel at that id directly (kind:39000 via fetchOpenChannel) and skip the starter if it is archived. Every other case still rethrows so setup stays recoverable. Also make the scope-fence test able to fail: it now flips the scope on the reload after the last starter's refusal, so deleting the post-reload fence fails it. relayErrorMessage also strips the relay's error:, rate-limited: and auth-required: prefixes. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Logan Johnson <loganj@squareup.com>
This commit is contained in:
@@ -84,6 +84,7 @@ final _inviteRelayConnectedProvider = FutureProvider.family<void, String>((
|
||||
/// App-level bridge from invite joining to the channels feature.
|
||||
class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
final Future<List<Channel>> Function() _loadChannels;
|
||||
final Future<Channel?> Function(String channelId) _fetchChannel;
|
||||
final Future<Channel> Function({
|
||||
required String channelId,
|
||||
required String name,
|
||||
@@ -97,9 +98,11 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
final String _relayHttpOrigin;
|
||||
final bool Function() _isScopeCurrent;
|
||||
|
||||
/// Creates recovery from channel-loading, creation, and join operations.
|
||||
/// Creates recovery from channel-loading, single-channel lookup, creation,
|
||||
/// and join operations.
|
||||
MobileInviteJoinRecovery({
|
||||
required Future<List<Channel>> Function() loadChannels,
|
||||
required Future<Channel?> Function(String channelId) fetchChannel,
|
||||
required Future<Channel> Function({
|
||||
required String channelId,
|
||||
required String name,
|
||||
@@ -113,6 +116,7 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
required String relayHttpOrigin,
|
||||
bool Function()? isScopeCurrent,
|
||||
}) : _loadChannels = loadChannels,
|
||||
_fetchChannel = fetchChannel,
|
||||
_createChannel = createChannel,
|
||||
_joinChannel = joinChannel,
|
||||
_relayHttpOrigin = relayHttpOrigin,
|
||||
@@ -124,8 +128,9 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
/// A partial setup remains recoverable, so a failed operation deliberately
|
||||
/// propagates to the invite flow instead of being reported as success. The
|
||||
/// one exception is a starter whose name or id the relay reports as taken
|
||||
/// by a channel the directory shows cannot serve as the starter: that is
|
||||
/// permanent, so the starter is skipped instead of blocking the join.
|
||||
/// by a channel the directory shows cannot serve as the starter, or an
|
||||
/// archived channel at the starter's own id that the directory omits: both
|
||||
/// are permanent, so the starter is skipped instead of blocking the join.
|
||||
@override
|
||||
Future<String?> ensureStarterChannels() async {
|
||||
_ensureScopeCurrent();
|
||||
@@ -162,9 +167,19 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery {
|
||||
.where((candidate) => candidate.id == channelId)
|
||||
.firstOrNull ??
|
||||
(nameTaken ? _findListedNamesake(channels, starter.slug) : null);
|
||||
// A holder the directory does not show yet may still be usable, so
|
||||
// keep the setup recoverable.
|
||||
if (holder == null) rethrow;
|
||||
if (holder == null) {
|
||||
// The directory omits archived channels, yet the relay still
|
||||
// keeps the id (desktop uses the same one), so a duplicate with
|
||||
// no match may be an archived starter. Read that id directly.
|
||||
if (!nameTaken && _isDuplicateChannelError(error)) {
|
||||
final atId = await _fetchChannel(channelId);
|
||||
_ensureScopeCurrent();
|
||||
if (atId != null && atId.isArchived) continue;
|
||||
}
|
||||
// A holder not visible yet may still be usable, so keep the setup
|
||||
// recoverable.
|
||||
rethrow;
|
||||
}
|
||||
// The relay keeps a channel's id and a listed channel's name, archived
|
||||
// ones included, so an unusable holder never frees the starter. Skip
|
||||
// it rather than strand the invite in permanent recovery.
|
||||
@@ -274,6 +289,14 @@ InviteJoinRecovery buildMobileInviteJoinRecovery(
|
||||
ensureScopeCurrent();
|
||||
return channels;
|
||||
},
|
||||
fetchChannel: (channelId) async {
|
||||
ensureScopeCurrent();
|
||||
final channel = await ref
|
||||
.read(channelsProvider.notifier)
|
||||
.fetchOpenChannel(channelId);
|
||||
ensureScopeCurrent();
|
||||
return channel;
|
||||
},
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
|
||||
@@ -81,14 +81,14 @@ RelayClosedClass classifyRelayClosed(String message) {
|
||||
}
|
||||
|
||||
final _relayErrorPrefixPattern = RegExp(
|
||||
r'^(?:exception:\s*)?(?:(?:invalid|blocked|restricted|duplicate|pow|unsupported):\s*)?',
|
||||
r'^(?:exception:\s*)?(?:(?:invalid|blocked|restricted|duplicate|pow|unsupported|error|rate-limited|auth-required):\s*)?',
|
||||
caseSensitive: false,
|
||||
);
|
||||
|
||||
/// User-facing text for a failed relay publish or request.
|
||||
///
|
||||
/// Drops the Dart `Exception: ` wrapper and a leading NIP-01 machine prefix
|
||||
/// (`invalid: `, `blocked: `, ...) so a relay refusal such as
|
||||
/// (`invalid: `, `blocked: `, `error: `, ...) so a relay refusal such as
|
||||
/// `invalid: an open channel named "Launch" already exists` reads as
|
||||
/// `An open channel named "Launch" already exists`.
|
||||
String relayErrorMessage(Object error) {
|
||||
|
||||
@@ -258,6 +258,7 @@ void main() {
|
||||
visibility: 'private',
|
||||
),
|
||||
],
|
||||
fetchChannel: (_) async => fail('no direct lookup expected'),
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
@@ -284,6 +285,7 @@ void main() {
|
||||
final created = <String, Map<String, Object?>>{};
|
||||
final recovery = MobileInviteJoinRecovery(
|
||||
loadChannels: () async => const [],
|
||||
fetchChannel: (_) async => fail('no direct lookup expected'),
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
@@ -342,6 +344,7 @@ void main() {
|
||||
_channel(id: 'relay-welcome', name: 'welcome-everyone'),
|
||||
];
|
||||
},
|
||||
fetchChannel: (_) async => fail('no direct lookup expected'),
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
@@ -382,19 +385,29 @@ void main() {
|
||||
({
|
||||
List<String> created,
|
||||
List<String> joined,
|
||||
List<String> lookups,
|
||||
MobileInviteJoinRecovery recovery,
|
||||
int Function() loads,
|
||||
})
|
||||
starterSetup({
|
||||
required List<Channel> Function(int load) channelsOnLoad,
|
||||
required String generalError,
|
||||
required String refusalError,
|
||||
Channel? atId,
|
||||
Set<String> refused = const {'general'},
|
||||
bool Function()? isScopeCurrent,
|
||||
void Function()? onLookup,
|
||||
}) {
|
||||
final created = <String>[];
|
||||
final joined = <String>[];
|
||||
final lookups = <String>[];
|
||||
var loadCount = 0;
|
||||
final recovery = MobileInviteJoinRecovery(
|
||||
loadChannels: () async => channelsOnLoad(++loadCount),
|
||||
fetchChannel: (channelId) async {
|
||||
lookups.add(channelId);
|
||||
onLookup?.call();
|
||||
return atId;
|
||||
},
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
@@ -405,7 +418,9 @@ void main() {
|
||||
ttlSeconds,
|
||||
}) async {
|
||||
created.add(name);
|
||||
if (name == 'general') throw Exception(generalError);
|
||||
if (refused.contains(name)) {
|
||||
throw Exception(refusalError);
|
||||
}
|
||||
return _channel(id: channelId, name: name, isMember: true);
|
||||
},
|
||||
joinChannel: (channelId) async => joined.add(channelId),
|
||||
@@ -415,6 +430,7 @@ void main() {
|
||||
return (
|
||||
created: created,
|
||||
joined: joined,
|
||||
lookups: lookups,
|
||||
recovery: recovery,
|
||||
loads: () => loadCount,
|
||||
);
|
||||
@@ -447,7 +463,7 @@ void main() {
|
||||
for (final (holder, error) in unusableHolders) {
|
||||
final setup = starterSetup(
|
||||
channelsOnLoad: (_) => [holder],
|
||||
generalError: error,
|
||||
refusalError: error,
|
||||
);
|
||||
|
||||
final focusChannelId = await setup.recovery.ensureStarterChannels();
|
||||
@@ -461,11 +477,69 @@ void main() {
|
||||
expect(focusChannelId, welcomeId, reason: holder.id);
|
||||
}
|
||||
|
||||
// An archived channel at the starter's own id, which the directory does
|
||||
// not return, is found by a direct lookup and skipped, not rethrown.
|
||||
final archivedAtId = starterSetup(
|
||||
channelsOnLoad: (_) => const [],
|
||||
refusalError: duplicateId,
|
||||
atId: _channel(
|
||||
id: generalId,
|
||||
name: 'general',
|
||||
archivedAt: DateTime.utc(2025),
|
||||
),
|
||||
);
|
||||
final focusAfterArchived = await archivedAtId.recovery
|
||||
.ensureStarterChannels();
|
||||
expect(archivedAtId.created, ['general', 'welcome-everyone']);
|
||||
expect(archivedAtId.lookups, [generalId]);
|
||||
expect(archivedAtId.joined, isEmpty);
|
||||
expect(archivedAtId.loads(), 2);
|
||||
expect(focusAfterArchived, welcomeId);
|
||||
|
||||
// The scope fence also holds across the direct lookup, checked on the
|
||||
// last starter so the next starter's own fence cannot mask it.
|
||||
var lookedUp = false;
|
||||
final fencedLookup = starterSetup(
|
||||
channelsOnLoad: (_) => const [],
|
||||
refusalError: duplicateId,
|
||||
refused: const {'welcome-everyone'},
|
||||
atId: _channel(
|
||||
id: welcomeId,
|
||||
name: 'welcome-everyone',
|
||||
archivedAt: DateTime.utc(2025),
|
||||
),
|
||||
isScopeCurrent: () => !lookedUp,
|
||||
onLookup: () => lookedUp = true,
|
||||
);
|
||||
await expectLater(
|
||||
fencedLookup.recovery.ensureStarterChannels(),
|
||||
throwsStateError,
|
||||
);
|
||||
expect(fencedLookup.lookups, [welcomeId]);
|
||||
|
||||
// Any other duplicate stays recoverable: nothing at the id, or a
|
||||
// channel there that is not archived, may be a starter still settling.
|
||||
for (final atId in [null, _channel(id: generalId, name: 'general')]) {
|
||||
final settling = starterSetup(
|
||||
channelsOnLoad: (_) => const [],
|
||||
refusalError: duplicateId,
|
||||
atId: atId,
|
||||
);
|
||||
await expectLater(
|
||||
settling.recovery.ensureStarterChannels(),
|
||||
throwsException,
|
||||
reason: '$atId',
|
||||
);
|
||||
expect(settling.created, ['general']);
|
||||
expect(settling.lookups, [generalId]);
|
||||
expect(settling.joined, isEmpty);
|
||||
}
|
||||
|
||||
// A taken name whose holder the directory does not show yet stays
|
||||
// recoverable: it may be a usable starter another client just created.
|
||||
final unseen = starterSetup(
|
||||
channelsOnLoad: (_) => const [],
|
||||
generalError: refusal,
|
||||
refusalError: refusal,
|
||||
);
|
||||
await expectLater(
|
||||
unseen.recovery.ensureStarterChannels(),
|
||||
@@ -474,33 +548,38 @@ void main() {
|
||||
expect(unseen.created, ['general']);
|
||||
expect(unseen.joined, isEmpty);
|
||||
expect(unseen.loads(), 2);
|
||||
expect(unseen.lookups, isEmpty);
|
||||
|
||||
// A usable open channel that appeared after the first load (lost race)
|
||||
// is joined rather than skipped.
|
||||
final raced = starterSetup(
|
||||
channelsOnLoad: (load) =>
|
||||
load == 1 ? const [] : [_channel(id: 'raced', name: 'general')],
|
||||
generalError: refusal,
|
||||
refusalError: refusal,
|
||||
);
|
||||
await raced.recovery.ensureStarterChannels();
|
||||
expect(raced.joined, ['raced']);
|
||||
|
||||
// The scope fence still holds after a refusal and its reload.
|
||||
// The reload after the last starter's refusal must be the one that
|
||||
// observes the change, so only the post-reload fence can catch it.
|
||||
var current = true;
|
||||
var fencedLoads = 0;
|
||||
final fenced = starterSetup(
|
||||
channelsOnLoad: (load) {
|
||||
if (load == 2) current = false;
|
||||
channelsOnLoad: (_) {
|
||||
if (++fencedLoads > 2) current = false;
|
||||
return [unusableHolders.first.$1];
|
||||
},
|
||||
generalError: refusal,
|
||||
refusalError: refusal,
|
||||
refused: const {'general', 'welcome-everyone'},
|
||||
isScopeCurrent: () => current,
|
||||
);
|
||||
await expectLater(
|
||||
fenced.recovery.ensureStarterChannels(),
|
||||
throwsStateError,
|
||||
);
|
||||
expect(fenced.created, ['general']);
|
||||
expect(fenced.loads(), 2);
|
||||
expect(fenced.created, ['general', 'welcome-everyone']);
|
||||
expect(fenced.loads(), 3);
|
||||
expect(fenced.joined, isEmpty);
|
||||
},
|
||||
);
|
||||
@@ -511,6 +590,7 @@ void main() {
|
||||
loadChannels: () async => [
|
||||
_channel(id: 'welcome-id', name: 'welcome-everyone'),
|
||||
],
|
||||
fetchChannel: (_) async => fail('no direct lookup expected'),
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
@@ -637,6 +717,7 @@ void main() {
|
||||
_channel(id: 'general-id', name: 'general'),
|
||||
_channel(id: 'welcome-id', name: 'welcome-everyone'),
|
||||
],
|
||||
fetchChannel: (_) async => fail('no direct lookup expected'),
|
||||
createChannel:
|
||||
({
|
||||
required channelId,
|
||||
|
||||
@@ -50,6 +50,9 @@ void main() {
|
||||
'invalid: an open channel named "Launch" already exists':
|
||||
'An open channel named "Launch" already exists',
|
||||
'blocked: community policy': 'Community policy',
|
||||
'error: internal failure': 'Internal failure',
|
||||
'rate-limited: slow down, retry in 3s': 'Slow down, retry in 3s',
|
||||
'auth-required: sign in first': 'Sign in first',
|
||||
'relay unavailable': 'Relay unavailable',
|
||||
'Event rejected': 'Event rejected',
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user