fix: pass loadSchema callback to pickSchemaDialect#1176
fix: pass loadSchema callback to pickSchemaDialect#1176datho7561 merged 4 commits intoredhat-developer:mainfrom
Conversation
|
Confirmed this is a problem. There's a build error caused by formatting that prettier doesn't like. I'll fix that locally and make sure that this fix works. |
|
Once the build error is fixed, I think this'll be good to merge. |
|
The tests seem to pass even without the fix; I'll try to figure out what's going on. |
pickSchemaDialect is a module-level function but used `this.loadSchema(s)` on line 1338, where `this` is undefined (strict mode). This caused a TypeError when resolving schemas with custom (non-standard) $schema URIs that require loading the meta-schema to infer the dialect. The fix adds an optional `loadSchema` callback parameter. The caller in `resolveSchemaContent` (a class method with proper `this` context) passes `this.loadSchema.bind(this)` via a closure variable. When `loadSchema` is not provided, the function returns `undefined` for unknown dialects instead of crashing.
Signed-off-by: David Thompson <davthomp@redhat.com>
f343c1a to
80a3500
Compare
- Add an await for a promise that was left dangling in order to properly propagate the error. - Remove other test suite; the test was passing with and without the fix. Signed-off-by: David Thompson <davthomp@redhat.com>
It's timing out on macOS Signed-off-by: David Thompson <davthomp@redhat.com>
That turned into a day long sidequest |
datho7561
left a comment
There was a problem hiding this comment.
I've modified the test case so that it fails without the fix, and left just the case where we check the broken behaviour.
@elasticjava Sorry for modifying your PR so much. I'd love another look at my work if you have time. If not, I'll go ahead and merge.
Thank you @datho7561 — really appreciate the improvements. The new Also great catch on the missing Thanks for merging! |
Summary
pickSchemaDialectis a module-level function but referencesthis.loadSchema(s)(line 1338), wherethisisundefinedin strict mode. This causes aTypeErrorwhen resolving schemas with custom (non-standard)$schemaURIs that require loading the meta-schema to infer the base dialect.Root cause: The function was introduced as part of #1166 (Support JSON Schema 2019 and 2020). It lives at module scope but uses
thisas if it were a class method.Fix: Add an optional
loadSchemacallback parameter. The caller inresolveSchemaContentpassesthis.loadSchema.bind(this)through a closure variable. WhenloadSchemais not provided, the function returnsundefinedfor unknown dialects instead of crashing.Changed lines: +9/-3 in
yamlSchemaService.tsReproduction
Any schema with a custom
$schemaURI that is not one of the well-known drafts triggers the bug:{ "$schema": "https://example.com/my-custom-meta-schema/v1", "type": "object", "properties": { "name": { "type": "string" } } }Also affects schemas embedding K8s definitions with
"$schema": "http://json-schema.org/schema#"in nested$defs.Test plan
pickSchemaDialect.test.tswith 4 test cases:$schemaURI does not throw TypeError$schemaon sub-definitions does not crash