posit-dev / publisher

MIT License
3 stars 0 forks source link

Better error message for content validation failure #1736

Closed mmarchetti closed 2 months ago

mmarchetti commented 2 months ago

Intent

Changes the error message raised on a validation failure to eliminate the ambiguity about the source of the 500 status (is it from Publisher or Connect?). Provide a direct link to the Connect logs in the Posit Publisher Logs panel when deployment fails.

Fixes #1726

Type of Change

Approach

Reference Connect logs in the error message. Provide a link to them in the publisher logs.

It's still a little odd, because the message in the error window says to see the Connect logs, and it has a Show Logs button that takes you to the publisher logs.

Automated Tests

Updated existing tests.

Directions for Reviewers

Deploy successful content, and content that can't start up successfully (raises an exception on app startup).

sagerb commented 2 months ago

I was testing an R projects, to find one which would not deploy, and it feels like I hit the scenario you were attempting to address:

2024-05-28 at 2 45 PM

but the unexpected response is shown in the logs under uploading files, which seems wrong.

When I look to the agent log, it seems like we were partially catching it.

2024-05-28 at 2 43 PM

Does this confirm your expectations?

mmarchetti commented 2 months ago

This PR is only trying to address the specific case of deployed content that can't run successfully. Improving handling for errors earlier in the process probably falls under #1257.

mmarchetti commented 2 months ago

The simplest test case I used to reproduce was to modify fastapi-simple/simple.py by adding this as the first line:

raise Exception("this will stop the app from starting up")
sagerb commented 2 months ago

Perhaps at one point, we can do something better in what is recorded in the deployment record for last error?

2024-05-28 at 2 50 PM

mmarchetti commented 2 months ago

Perhaps at one point, we can do something better in what is recorded in the deployment record for last error?

Yes; that error needs to be more specific. It's a Connect API call failure, and those usually include a JSON payload with an error message.