search

eclipse-cdt-cloud / trace-server-protocol

Specification of the Trace Server Protocol (TSP)
https://eclipse-cdt-cloud.github.io/trace-server-protocol/
Apache License 2.0
23 stars 16 forks source link

Errors are specified as strings, while clients expect json #43

Open tahini opened 3 years ago

tahini commented 3 years ago

the tsp-typescript-client tries to convert to json any response they get from the server. But some of the error messages, like when putting an invalid trace, return as text strings.

This causes like this in the theia-trace-extension:

Uncaught (in promise) SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data openTrace tsp-client.ts:55 tryOpen trace-manager.ts:76

We should either:

1- Document all error messages to be json, like {message: string} in the protocol and have the server return them this way

or

2- Have the tsp-typescript-client verify the response code before trying to convert to json and have the consumers do likewise whe processing responses

What do you think?

MatthewKhouzam commented 3 years ago

I would hard code it as JSON at the moment. It makes sense to have structured errors.

Thoughts?

tahini commented 3 years ago

+1 for json messages. For now, we can have the server return json messages and update the protocol soon-ish

PatrickTasse commented 3 years ago

Related pull request in tsp-typescript-client: https://github.com/theia-ide/tsp-typescript-client/pull/15. With that change it's OK not to have a json model in the response, and the plain text response will be available to the caller.

tahini commented 3 years ago

Related PR in trace server: https://git.eclipse.org/r/c/tracecompass.incubator/org.eclipse.tracecompass.incubator/+/173840

Additional failsafe in tsp client is good