solid-client-js
solid-client-js copied to clipboard
Added flag to control diagnostic output on errors
New feature description
Added flag to control diagnostic output on errors (as this output can be distracting when errors are expected).
Checklist
- [X] All acceptance criteria are met.
- [X] Relevant documentation, if any, has been written/updated.
- [X] The changelog has been updated, if applicable.
- [X] New functions/types have been exported in
index.ts, if applicable. - [X] New modules (i.e. new
.tsfiles) are listed in theexportsfield inpackage.json, if applicable. - [X] New modules (i.e. new
.tsfiles) are listed in thetypedocOptions.entryPointsfield intsconfig.json, if applicable. - [X] Commits in this PR are minimal and have descriptive commit messages.
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Yep: "What's the use case for suppressing part of the error message (what situations are the errors expected)?" The diagnostic part of the output contains the entire contents of the dataset as Markdown, so it's very noisy (it's useful output, generally, hence the flag is defaulted to 'true'). An expected error case is when doing ETL - we want the Loading-to-a-Pod process to be idempotent, so a resource may already exist (in which case we add any new triples to that resource), or may not exist at all - so a 404 error could be expected, normal behaviour.
"Should this flag be added to internal_defaultFetchOptions, or is it only relevant for some requests?" Yep - absolutely. It was just that TypeScript was complaining when I tried to do that - whereas this code 'just worked'. But I'd be very happy to rework to simplify.
Just marking this PR as draft, as after discussion with Andy today, we can make this cleaner by added the new flag to the internal_defaultFetchOptions object, but we should also add a proper TypeScript type too, and then change all references to typeof internal_defaultFetchOptions to use the type instead, and then use spreads to pull defaults, e.g.,:
const config = {
...internal_defaultFetchOptions,
...options,
};
Some places also use options: Partial<typeof internal_defaultFetchOptions>, which means all properties are optional. That means we could use that more widely and don’t necessarily need to create a separate ‘type’ (but we do still need to make sure everywhere handles missing values for fetch with the config).
So given there's a few options, and it needs careful consideration, flipping this to 'draft' (but the SDK could still merge this code to get it working now, and then tidy up everything properly in a separate ticket).
This might be better solved by having a custom error type with a captureDiagnostics() method on it, which would then generate the extend output; keeping error messages smaller by default. (That would also put the control in the developers' hands, and would mean potentially sensitive information wouldn't end up by default in error reporting tools like Sentry.
Closing due to this being heavily out of date, and before we add anything similar, we first need a Design Document for the feature.