Octoprint: api/job response `status` field gets polluted by python exception message

[MarcinOrlowski]

What were you doing?

I am GET requesting api/job from API and the state field of the response for the printer that I pulled USB cable from the printer (but not from the Raspberry) and the response looks polluted by python exception message, instead of just read Offline:

{
  "job": {
    "averagePrintTime": null,
    "estimatedPrintTime": null,
    "filament": null,
    "file": {
      "date": null,
      "display": null,
      "name": null,
      "origin": null,
      "path": null,
      "size": null
    },
    "lastPrintTime": null,
    "user": null
  },
  "progress": {
    "completion": null,
    "filepos": null,
    "printTime": null,
    "printTimeLeft": null,
    "printTimeLeftOrigin": null
  },
  "state": "Offline (Error: SerialException: 'device reports readiness to read but returned no data (device disconnected or multiple access on port?)' @ comm.py:_readline:2916)"
}

1. Start Octoprint
2. Disconnect the printer by pulling the usb cable from the printer
3. Request api/job status
4. See error

What did you expect to happen?

The state field should just read Offline

Did the same happen when running OctoPrint in safe mode?

Yes, with slightly different message though:

{
  "job": {
    "averagePrintTime": null,
    "estimatedPrintTime": null,
    "filament": null,
    "file": {
      "date": null,
      "display": null,
      "name": null,
      "origin": null,
      "path": null,
      "size": null
    },
    "lastPrintTime": null,
    "user": null
  },
  "progress": {
    "completion": null,
    "filepos": null,
    "printTime": null,
    "printTimeLeft": null,
    "printTimeLeftOrigin": null
  },
  "state": "Offline (Error: No more candidates to test, and no working port/baudrate combination detected.)"
}

Versions

OctoPrint 1.4.2 Python 2.7.16 OctoPi 0.17.0

Link to contents of terminal tab or serial.log

Changing monitoring state from "Offline" to "Detecting serial connection"
Performing autodetection with 0 port/baudrate candidates: 
Changing monitoring state from "Detecting serial connection" to "Error: No more candidates to test, and no working port/baudrate combination detected."
Changing monitoring state from "Error: No more candidates to test, and no working port/baudrate combination detected." to "Offline (Error: No more candidates to test, and no working port/baudrate combination detected.)"

I have read the FAQ.

[foosel]

That is intended and correct behaviour. state contains "A textual representation of the current state of the job". The API documentation makes no claims to a restricted value range here, note how it says "i.e.", not "one of":

However, I agree that excessive exception information here doesn't really add value, so I'll restrict it to exception type and message in case of an error. I will not fully remove this as it's proven a valuable debugging tool in the past when all users provide are screenshots from the state panel, which gets what it displays from the same source as the API.

If you need a machine readable value range for the state, I suggest to use the printer state endpoint: https://docs.octoprint.org/en/master/api/printer.html#get--api-printer & https://docs.octoprint.org/en/master/api/datamodel.html#sec-api-datamodel-printer-state

[MarcinOrlowski]

Let me suggest slight, backward compatible refactoring here. First, make state returning known values from fixed set (said. "Offline", "Error" etc) for pure machine processing. No additional information - just state. Then add new field i.e. state_msg that would contain any additional information (i.e. said exception message) that may accompany the state. UI would need small change to include state_msg along state but that's mostly it. And if you'd keep currently used words for state then it will be 100% backward compatible.

[MarcinOrlowski]

Let me also point that documentation you quote should explicit state it can contain more than one word, best with clear example. The i.e. thing is NOT indicating that. I'd bet that vast majority of readers gets the picture that there can be more state returned but these all be single worded.

[foosel]

Done with the above commit, ready for 1.6.0 (1.5.0 is already frozen and in RC phase)

$ curl http://localhost:5000/api/job?apikey=[redacted]
{
  "error": "SerialException: ClearCommError failed (PermissionError(13, 'The device does not recognize the command.', None, 22))",
  "job": {
    "averagePrintTime": null,
    "estimatedPrintTime": null,
    "filament": null,
    "file": {
      "date": null,
      "display": null,
      "name": null,
      "origin": null,
      "path": null,
      "size": null
    },
    "lastPrintTime": null,
    "user": null
  },
  "progress": {
    "completion": null,
    "filepos": null,
    "printTime": null,
    "printTimeLeft": null,
    "printTimeLeftOrigin": null
  },
  "state": "Offline after error"
}

https://docs.octoprint.org/en/maintenance/api/job.html#job-information-response

[MarcinOrlowski]

"state": "Offline after error"

API should not talk English in first place - I mean it should be machine-to-machine talk first (so easy to process in code) and as predictable as possible. IMHO, if there'd be anything to say to humans then it should be said by API client app (based on API response). And if for any reason API wants to say something directly (like that error message) then if should go in separate field so API client just display it w/o any processing. Now we still have this mixed.

Also about Offline after error - does it mean there can also be just Offline state too? Shuld be. But then what is Offline after error really for if you can have Offline state and non empty error message. Doesn't it mean exactly Offline after error?

[cp2004]

States of offline are either 'user has pressed disconnect' > offline , or an error has occured causing the disconnect > offline after error.

IMHO the part about APIs not taking to humans doesn't sit quite right with me - since 'Offline after error' fits the idea of 'textual representation' quite well. Otherwise, you are significantly complicating working out whether the state is offline or offline after error, since you have to check if there is an error if state is offline - rather than just displaying the state, exactly as it is received.

It might help for you to suggest a use case where this is important, so we can work to find out a better solution.

[foosel]

API should not talk English in first place

Then use the printer state where you get convenient well defined boolean state flags (and a clear HTTP status code in case of not being connected). The state on the job endpoint was added more as a shortcut for clients who want something quick to display to the user when consuming this API, not a full blown state model replacement - as I said, that already exists on the printer state endpoint which should be consumed if the state is needed for logic implementation (and those flags - as pushed on the websocket - are exactly what the core UI uses for state logic).

[MarcinOrlowski]

Otherwise, you are significantly complicating working out whether the state is offline or offline after error, since you have to check if there is an error if state is offline

@cp2004 you see this wrong way. Current implementation returns status and error. Why it's even called error? Why not generic name i.e. status_msg? If one day any other state than error would need to be accompanied by extra message - taadam - you put it there and that's it. You need to change nothing.

was added more as a shortcut for clients who want something

@foosel and that's how you create technological debt :)

Then use the printer state where you get convenient well defined boolean state flags

I understand that majority of users (and devs) prefer plugins over API. But current API needs more love and corrections. I got a feeling API is not really designed at all. It was just written as it was needed and sits there. BTW: what about introducing API versioning?

Getting back to printer state. That endpoint is also broken :( When printer is connected all is fine and you will get a JSON object, but once disconnect it then you get slapped with text message! Just plain text Printer is not operational. That's BAD. And it was me who was"significantly complicating" because I suggested that instead of displaying one string one now would unconditionally concatenate two fields then display :) C'mon.

[jneilliii]

Getting back to printer state. That endpoint is also broken :( When printer is connected all is fine and you will get a JSON object, but once disconnect it then you get slapped with text message! Just plain text Printer is not operational. That's BAD. And it was me who was"significantly complicating" because I suggested that instead of displaying one string one now would unconditionally concatenate two fields then display :) C'mon.

The response code to this failure returns different though in this case.

[MarcinOrlowski]

The response code

@jneilliii "the response code" is what exactly? HTTP return code? API hard-depending on underlying transport layer is fundamentally broken.

[jneilliii]

An http status code, as defined in the rfc2616 and documented in the REST API.

https://docs.octoprint.org/en/master/api/printer.html#retrieve-the-current-printer-state

[foosel]

As always when you perceive something to be suboptimal or outright broken in an open source project that you care about enough to demand changes in, you are welcome to contribute to make it better.

I frankly don't have the time for yet another big construction site for the foreseeable future, but I'm open to suggestions in form of actual contributions. So sit down and draw up a suggestion for a new version and we can talk about it.

And in general I would prefer if you showed some more consideration in your phrasing. There are better ways to criticize a status quo that works just fine for a whole existing ecosystem than categorically declaring it as "broken" and "not designed at all", or lecturing the maintainer on well understood risks and realities of maintaining a large codebase for several years.

Tldr: contribute to make it better, stop complaining or don't use it - the choice is yours.