fix(save-attachments): authoritative disk-verify + hide extension in rename UI

Two bugs reported after v1.1.2:

1. False-negative "נכשלו" toast even though the file was on disk. Root
   cause from the log: Polly's 15s timeout on /NetworkStorage/action/upload
   fires while the server is still writing larger PDFs, so the client sees
   TimeoutRejectedException → Outcome.Failed, while the server quietly
   finishes the write. The outcome is now decided by listing the target
   folder, not by the upload response. If the upload exception fires, the
   listing is retried up to 4 times with 2s delays to let the server
   finish. Saved if the file appears, Failed only if it never does.

2. The filename TextBox in the attachments list exposed the extension
   (.pdf / .docx). Users have to delete it manually when renaming and
   risk dropping it. AttachmentRowViewModel now splits FileName into
   BaseName (editable) and Extension (frozen at construction). The XAML
   binds the TextBox to BaseName and shows the Extension as a muted
   adjacent chip. The on-disk filename = BaseName + Extension always.

Bump to 1.1.3.0 for the patch release.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
PointStar
2026-05-24 19:21:28 +03:00
parent 18b6640e57
commit b1dc57324e
5 changed files with 150 additions and 68 deletions
@@ -46,35 +46,17 @@ namespace MarcusLaw.OutlookAddin.Core.Services
continue;
}
string? attachmentId = null;
Exception? uploadException = null;
// Step 1: create the EspoCRM Attachment (binary stored in
// data/upload/<id>; not yet on the network share). Auth
// failures here are terminal — skip remaining items.
try
{
// Step 1: create the EspoCRM Attachment (binary stored
// in data/upload/<id>; not yet on the network share).
var uploaded = await _client.UploadAttachmentAsync(
att.Name, att.ContentType, att.ContentBase64, cancellationToken).ConfigureAwait(false);
// Step 2: ask Marcus-Law's NetworkStorageIntegration
// to place the uploaded blob under <targetFolderPath>.
// The integration appends the filename to the path.
await _client.UploadAttachmentToNetworkStorageAsync(
uploaded.Id, trimmedFolder, cancellationToken).ConfigureAwait(false);
// Step 3: verify the file actually landed on the share —
// the server can return 200 yet leave nothing on disk
// when a downstream WebDAV proxy silently fails. Read
// the folder back and look for our filename.
await VerifyArrivedAsync(att.Name, trimmedFolder, cancellationToken).ConfigureAwait(false);
_logger.Information(
"Saved attachment {Name} → {Path} (attachmentId={AttachmentId})",
att.Name, trimmedFolder, uploaded.Id);
summary.Items.Add(new AttachmentSaveItemResult
{
FileName = att.Name,
Outcome = AttachmentSaveOutcome.Saved,
AttachmentId = uploaded.Id
});
attachmentId = uploaded.Id;
}
catch (EspoCrmAuthorizationException ex)
{
@@ -86,20 +68,85 @@ namespace MarcusLaw.OutlookAddin.Core.Services
Outcome = AttachmentSaveOutcome.AuthRequired,
ErrorMessage = ex.Message
});
continue;
}
catch (OperationCanceledException)
{
throw;
}
catch (OperationCanceledException) { throw; }
catch (Exception ex)
{
_logger.Warning(ex, "Attachment save failed for {Name}", att.Name);
// No attachment ID means we have nothing on the share —
// a hard fail.
_logger.Warning(ex, "Attachment upload (Step 1) failed for {Name}", att.Name);
summary.Items.Add(new AttachmentSaveItemResult
{
FileName = att.Name,
Outcome = AttachmentSaveOutcome.Failed,
ErrorMessage = ex.Message
});
continue;
}
// Step 2: ask the NetworkStorageIntegration to place the
// uploaded blob under <targetFolderPath>. The 15s Polly
// timeout sometimes fires while the server is still
// writing the file — capture the exception but don't
// decide the outcome yet.
try
{
await _client.UploadAttachmentToNetworkStorageAsync(
attachmentId, trimmedFolder, cancellationToken).ConfigureAwait(false);
}
catch (EspoCrmAuthorizationException ex)
{
_logger.Warning(ex, "Attachment save aborted on Step 2: EspoCRM rejected credentials");
summary.AuthRequired = true;
summary.Items.Add(new AttachmentSaveItemResult
{
FileName = att.Name,
Outcome = AttachmentSaveOutcome.AuthRequired,
ErrorMessage = ex.Message
});
continue;
}
catch (OperationCanceledException) { throw; }
catch (Exception ex)
{
uploadException = ex;
_logger.Warning(ex,
"Attachment NetworkStorage upload (Step 2) reported failure for {Name}; will verify disk before declaring failure",
att.Name);
}
// Step 3: verify on disk. Authoritative — if the file is
// there, the save succeeded regardless of what Step 2
// reported (Polly timeouts during large writes are common).
var verified = await VerifyArrivedWithRetryAsync(
att.Name, trimmedFolder, uploadException != null, cancellationToken).ConfigureAwait(false);
if (verified)
{
_logger.Information(
"Saved attachment {Name} → {Path} (attachmentId={AttachmentId}, uploadException={HadException})",
att.Name, trimmedFolder, attachmentId, uploadException != null);
summary.Items.Add(new AttachmentSaveItemResult
{
FileName = att.Name,
Outcome = AttachmentSaveOutcome.Saved,
AttachmentId = attachmentId
});
}
else
{
var message = uploadException?.Message
?? "Server reported success but the file is not on the share.";
_logger.Warning(
"Attachment save Failed for {Name}: not present after verify (uploadException={Msg})",
att.Name, message);
summary.Items.Add(new AttachmentSaveItemResult
{
FileName = att.Name,
Outcome = AttachmentSaveOutcome.Failed,
ErrorMessage = message
});
}
}
@@ -107,51 +154,60 @@ namespace MarcusLaw.OutlookAddin.Core.Services
}
/// <summary>
/// Post-upload sanity check: list the folder we just wrote into
/// and confirm a file with the same (possibly sanitized) name
/// shows up. Diagnostic only — does not change the outcome. If
/// the file isn't there, log a WARNING with everything that IS
/// in the folder so we can spot the actual server-side path.
/// Authoritative post-upload check. Returns true if the file is on
/// disk under <paramref name="folderPath"/>. When the upload call
/// raised (typically Polly's 15s timeout on a still-writing server),
/// we retry the listing a few times so the server gets a chance
/// to finish.
/// </summary>
private async Task VerifyArrivedAsync(string expectedName, string folderPath, CancellationToken ct)
private async Task<bool> VerifyArrivedWithRetryAsync(string expectedName, string folderPath, bool afterFailure, CancellationToken ct)
{
// When the upload appeared to succeed, one check is plenty — the
// file is already there. When it appeared to fail, the server
// may still be writing; give it a few extra seconds.
var attempts = afterFailure ? 4 : 1;
var delay = TimeSpan.FromSeconds(2);
for (int i = 0; i < attempts; i++)
{
ct.ThrowIfCancellationRequested();
if (i > 0) await Task.Delay(delay, ct).ConfigureAwait(false);
if (await IsPresentAsync(expectedName, folderPath, ct).ConfigureAwait(false))
{
if (i > 0)
{
_logger.Information(
"Post-upload verify: '{Expected}' appeared under '{Path}' on attempt {Attempt}/{Total} despite upload exception",
expectedName, folderPath, i + 1, attempts);
}
return true;
}
}
return false;
}
private async Task<bool> IsPresentAsync(string expectedName, string folderPath, CancellationToken ct)
{
try
{
var items = await _client.ListNetworkStorageFolderAsync(folderPath, ct).ConfigureAwait(false);
var trimmedExpected = expectedName.Trim().TrimStart('-', ' ').TrimEnd('-', ' ');
bool found = false;
foreach (var it in items)
{
if (it.IsFolder) continue;
if (string.Equals(it.Name, expectedName, StringComparison.Ordinal) ||
string.Equals(it.Name, trimmedExpected, StringComparison.Ordinal))
{
found = true;
break;
return true;
}
}
if (!found)
{
var names = new List<string>();
foreach (var it in items)
{
if (it.IsFolder) continue;
names.Add(it.Name);
}
_logger.Warning(
"Post-upload verify: expected '{Expected}' under '{Path}' but it's NOT in the listing. " +
"Server reported the upload as successful but the file is not on the share. " +
"Files currently in that folder: [{Files}]",
expectedName, folderPath, string.Join(", ", names));
}
else
{
_logger.Information("Post-upload verify: '{Expected}' present under '{Path}'", expectedName, folderPath);
}
return false;
}
catch (Exception ex)
{
_logger.Warning(ex, "Post-upload verify failed for '{Expected}' under '{Path}'", expectedName, folderPath);
_logger.Warning(ex, "Post-upload listing under '{Path}' failed", folderPath);
return false;
}
}
}
@@ -184,14 +184,20 @@
FontSize="11"
VerticalAlignment="Center"
Margin="8,0,0,0" />
<TextBox Text="{Binding FileName, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"
<TextBlock Text="{Binding Extension}"
DockPanel.Dock="Left"
Foreground="{StaticResource Muted}"
FontSize="12"
VerticalAlignment="Center"
Margin="6,0,0,0" />
<TextBox Text="{Binding BaseName, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"
IsEnabled="{Binding IsSelected}"
VerticalAlignment="Center"
BorderThickness="1"
BorderBrush="{StaticResource Border}"
Padding="6,3"
Margin="8,0,0,0"
ToolTip="לחץ כדי לערוך את שם הקובץ לפני השמירה" />
ToolTip="לחץ כדי לערוך את שם הקובץ (הסיומת נשמרת אוטומטית)" />
</DockPanel>
</DataTemplate>
</ItemsControl.ItemTemplate>
@@ -1,3 +1,4 @@
using System.IO;
using CommunityToolkit.Mvvm.ComponentModel;
using MarcusLaw.OutlookAddin.Core.Models;
@@ -10,23 +11,42 @@ namespace MarcusLaw.OutlookAddin.UI.ViewModels
[ObservableProperty]
private bool isSelected = true;
public string FileName
/// <summary>
/// Frozen at construction from the original filename. The user
/// edits only the base name; the extension always tags along.
/// </summary>
public string Extension { get; }
/// <summary>
/// Editable base name without the extension. Setting this rewrites
/// <see cref="EspoAttachment.Name"/> as &lt;basename&gt;&lt;Extension&gt;.
/// </summary>
public string BaseName
{
get => Data.Name;
get => Path.GetFileNameWithoutExtension(Data.Name) ?? string.Empty;
set
{
var newName = value ?? string.Empty;
if (Data.Name == newName) return;
Data.Name = newName;
var newBase = (value ?? string.Empty).Trim();
var newFull = newBase + Extension;
if (Data.Name == newFull) return;
Data.Name = newFull;
OnPropertyChanged();
OnPropertyChanged(nameof(FileName));
}
}
/// <summary>
/// Full filename including extension — what actually goes to the
/// server. Read-only mirror of <see cref="EspoAttachment.Name"/>.
/// </summary>
public string FileName => Data.Name;
public string SizeDisplay { get; }
public AttachmentRowViewModel(EspoAttachment data)
{
Data = data;
Extension = Path.GetExtension(data.Name) ?? string.Empty;
SizeDisplay = ComputeSize(data.ContentBase64);
}
+1 -1
View File
@@ -36,7 +36,7 @@
<PublishUrl>C:\Users\Chaim\source\repos\OutlookAddin\Publish\</PublishUrl>
<InstallUrl>https://gitea.dev.marcus-law.co.il/espocrm-extensions/OutlookAddin/raw/branch/main/Publish/</InstallUrl>
<TargetCulture>en</TargetCulture>
<ApplicationVersion>1.1.2.0</ApplicationVersion>
<ApplicationVersion>1.1.3.0</ApplicationVersion>
<AutoIncrementApplicationRevision>false</AutoIncrementApplicationRevision>
<UpdateEnabled>true</UpdateEnabled>
<UpdateInterval>7</UpdateInterval>
+2 -2
View File
@@ -33,6 +33,6 @@ using System.Security;
// You can specify all the values or you can default the Build and Revision Numbers
// by using the '*' as shown below:
// [assembly: AssemblyVersion("1.0.*")]
[assembly: AssemblyVersion("1.1.2.0")]
[assembly: AssemblyFileVersion("1.1.2.0")]
[assembly: AssemblyVersion("1.1.3.0")]
[assembly: AssemblyFileVersion("1.1.3.0")]