Better error handling for download endpoint

[johnml1135]

In e2e test, grab current machine.py version

---

This change is 

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 66.52%. Comparing base (d295258) to head (2ed31da).

Files	Patch %	Lines
...tCore/Services/ServalTranslationEngineServiceV1.cs	0.00%	3 Missing ⚠️
...IL.Machine.AspNetCore/Services/NmtEngineService.cs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #173   +/-   ##
=======================================
  Coverage   66.52%   66.52%           
=======================================
  Files         437      437           
  Lines       34233    34233           
  Branches     4583     4583           
=======================================
  Hits        22775    22775           
  Misses      10368    10368           
  Partials     1090     1090           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ddaspit]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @johnml1135)

---

src/SIL.Machine.AspNetCore/Services/InvalidOperationInterceptor.cs line 3 at r1 (raw file):

namespace SIL.Machine.AspNetCore.Services;

public class InvalidOperationInterceptor : Interceptor

Is there a particular exception that we aren't handling that prompted the addition of this interceptor?

---

src/SIL.Machine.AspNetCore/Services/NmtEngineService.cs line 140 at r1 (raw file):

        if (!fileExists)
        {
            throw new RpcException(

You shouldn't throw an RpcException from the service layer, since it couples the services to the gRPC implementation of the API layer. You can catch the FileNotFoundException in the API layer and rethrow an RpcException. Also, the error message doesn't totally make sense to me. Maybe something like "The model for build revision , {engine.BuildRevision}, does not exist."

[johnml1135]

src/SIL.Machine.AspNetCore/Services/NmtEngineService.cs line 140 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

You shouldn't throw an RpcException from the service layer, since it couples the services to the gRPC implementation of the API layer. You can catch the FileNotFoundException in the API layer and rethrow an RpcException. Also, the error message doesn't totally make sense to me. Maybe something like "The model for build revision , {engine.BuildRevision}, does not exist."

Put back to FileNotFound

[johnml1135]

src/SIL.Machine.AspNetCore/Services/InvalidOperationInterceptor.cs line 3 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Is there a particular exception that we aren't handling that prompted the addition of this interceptor?

I put moved the FileNotFound catch to the API layer (I believe). I just looked up 3 layer architecture - I think it makes sense now.

[ddaspit]

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @johnml1135)

---

src/SIL.Machine.AspNetCore/Services/ServalTranslationEngineServiceV1.cs line 155 at r2 (raw file):

        catch (FileNotFoundException e)
        {
            throw new RpcException(new Status(StatusCode.DataLoss, e.Message));

I think NotFound would be a more appropriate status code.

[johnml1135]

src/SIL.Machine.AspNetCore/Services/ServalTranslationEngineServiceV1.cs line 155 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I think NotFound would be a more appropriate status code.

Sure - data loss has "not recoverable" - the person can rebuild the engine if need be, as could be the case if we allow for patching engines to allow for "IsModelPersisted" to be updated.

[ddaspit]

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)