Skip to content

Check HTTP response content type before trying to decode it as JSON #1196

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

Merged
merged 2 commits into from
Jun 14, 2025

Conversation

IsaacG
Copy link
Member

@IsaacG IsaacG commented Jun 13, 2025

No description provided.

@SleeplessByte
Copy link
Member

You should check for both this or ends with +json so that Jeremy can switch in the future to a +json type.

E.g.

application/problem+json

@IsaacG
Copy link
Member Author

IsaacG commented Jun 13, 2025

You should check for both this or ends with +json so that Jeremy can switch in the future to a +json type.

Done. I'm not familiar with that new format. Is the intend that you can put any arbitrary string there? application/<anything>+json?

@SleeplessByte
Copy link
Member

SleeplessByte commented Jun 13, 2025

Yes, common is making a media type in the vnd. (vendor) tree, e.g.

application/vnd.exercism.submission.v1+json

That could be specced to always give the same format for the JSON, instead of merely the JSON syntax. We may never do this, but it's safe to include, guarantees a JSON-syntaxed response and allows us to later decide.

GraphQL for example also has a specced response in their working draft of the spec. It guarantees not just JSON syntax, but also format.

Similar:

# XLS
application/vnd.ms-excel

# XLSX (open format)
application/vnd.openxmlformats-officedocument.spreadsheetml.sheet

It's vital for HATEOAS (a way of designing HTTP APIs as described in Roy Fieldings thesis), used by some of the big ones. Resource dump with surface level information: Don't be smart and use web standards. Example of implementation of type-checked JSON requests and responses.

@IsaacG IsaacG merged commit 05260ea into exercism:main Jun 14, 2025
7 checks passed
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

Successfully merging this pull request may close these issues.

3 participants