Skip to content

Provide an exception message for BadRef#272

Merged
lewisjkl merged 2 commits intodisneystreaming:mainfrom
Jacoby6000:fix-badref-error-messages
Aug 29, 2025
Merged

Provide an exception message for BadRef#272
lewisjkl merged 2 commits intodisneystreaming:mainfrom
Jacoby6000:fix-badref-error-messages

Conversation

@Jacoby6000
Copy link
Contributor

I ran in to this while translating some non-trivial json schema specs. Without this change, bad refs evaluate to null when getMessage is called.

Also updated the trait definition such that all traits must provide an error message or else compilation will fail.

@CLAassistant
Copy link

CLAassistant commented Aug 15, 2025

CLA assistant check
All committers have signed the CLA.

@lewisjkl
Copy link
Collaborator

Hey @Jacoby6000 thanks for contributing! I like the change, do you think you could also add a test case that shows what this fixes so we can avoid future regressions?

@Jacoby6000
Copy link
Contributor Author

I went ahead and added the test suite. In hindsight I'm not so sure this is actually testing anything, given that the compiler now enforces that messages must not be null

@lewisjkl
Copy link
Collaborator

I went ahead and added the test suite. In hindsight I'm not so sure this is actually testing anything, given that the compiler now enforces that messages must not be null

Right, I was more-so thinking of a test where the input was a reduced version of whatever you ran that caused the error to pop up originally. That way we have a test that, without your fix, would fail.

@Jacoby6000
Copy link
Contributor Author

Jacoby6000 commented Aug 15, 2025

a test where the input was a reduced version of whatever you ran that caused the error to pop up originally. That way we have a test that, without your fix, would fail.

Ah, that is addressed in #273

This PR doesn't really fix anything. It just improves the developer experience when they come across a BadRef

@lewisjkl lewisjkl merged commit bf37aaa into disneystreaming:main Aug 29, 2025
2 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