Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tahini opened this issue Dec 14, 2020 · 4 comments
Open

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

tahini opened this issue Dec 14, 2020 · 4 comments

Comments

@tahini
Copy link
Contributor

tahini commented Dec 14, 2020

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
Copy link

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

Thoughts?

@tahini
Copy link
Contributor Author

tahini commented Dec 15, 2020

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

@PatrickTasse
Copy link
Contributor

Related pull request in tsp-typescript-client: eclipse-cdt-cloud/tsp-typescript-client#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
Copy link
Contributor Author

tahini commented Dec 21, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants