Skip to content

fix: Dataloader helper Try classcast exception #177

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ameerabdul
Copy link

@ameerabdul ameerabdul commented Mar 10, 2025

Summary

  • Creating a new DataLoader with newMappedDataLoaderWithTry and batchingEnabled as false and cachingEnabled as true.
  • This will cause the value to which is already a Try instance to be wrapped again in another Try.succeed causing class cast exception

@ameerabdul ameerabdul marked this pull request as ready for review March 10, 2025 17:28
valuesInKeyOrder.set(listIndex, (Try<V>) v);
} else {
valuesInKeyOrder.set(listIndex, Try.succeeded(v));
}
}
List<V> assembledValues = valuesInKeyOrder.stream().map(Try::get).collect(toList());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be a bigger change as I can't go a Try.get on failed instances

@bbakerman
Copy link
Member

Can we please have a unit test showing this working.

Its not enough to fix the code, we need to lock in that behavior

@bbakerman
Copy link
Member

I managed to do a reproducton of this. I can confirm its a genuine problem but this is not the fix for it

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

Successfully merging this pull request may close these issues.

2 participants