Well, for one thing, display it in the Developer Mode console (or whatever other debugging UIs my app happens to have).
Jeff. On 01/07/2014 11:14, "Carsten Ziegeler" <cziege...@apache.org> wrote: >So if your adapter is buggy and you get an exception, what do you do with >it? > >Carsten > > >2014-07-01 12:08 GMT+02:00 Jeff Young <j...@adobe.com>: > >> Hi Carsten, >> >> Sure, but Konrad has a point in that I think sometimes the client *does* >> care why the adaption failed. For instance, if it had to do with >> something entirely different from whether or not adaption would normally >> work. >> >> Let's say that I have a resource that should adapt to XYZ, but that my >> adapter is currently buggy. I'd like to get an exception for that, but >> said exception is going to get eaten. >> >> I do agree that if I have a resource that should NOT adapt to XYZ, that >> getting back null is fine, and that I don't care why the adaption >>failed. >> >> Cheers, >> Jeff. >> >> >> On 01/07/2014 10:19, "Carsten Ziegeler" <cziege...@apache.org> wrote: >> >> >Sure :) For the adapter pattern, the client does not care why the >>adaption >> >failed, the client is just interested in the result (success or not) >> >Validation is a different beast, if validation fails you want to know >> >specific reasons why it failed - and this can be multiple. >> >I tried to explain in my first mail on this thread, that all other use >> >cases mentioned can be handled with the current implementation - with >>the >> >exception of validation. But I think validation requires a different >> >concept than the adapter pattern. >> > >> >Carsten >> > >> > >> >2014-07-01 11:09 GMT+02:00 Jeff Young <j...@adobe.com>: >> > >> >> Hi Carsten, >> >> >> >> Can you say more? (I'm not sure I understand what you're getting >> >>at....) >> >> >> >> Thanks, >> >> Jeff. >> >> >> >> >> >> On 01/07/2014 09:56, "Carsten Ziegeler" <cziege...@apache.org> wrote: >> >> >> >> >adaption and validation are different concerns >> >> > >> >> >Carsten >> >> > >> >> > >> >> >2014-07-01 10:55 GMT+02:00 Jeff Young <j...@adobe.com>: >> >> > >> >> >> We could solve that by defining a specific exception for >> >> >> adaptation-not-possible and then catch only that. >> >> >> >> >> >> Of course that would leak tons of exceptions from code written >>before >> >> >>that >> >> >> exception became available. Maybe do the catching based on some >> >>sort of >> >> >> version clue? >> >> >> >> >> >> Cheers, >> >> >> Jeff. >> >> >> >> >> >> >> >> >> On 01/07/2014 09:40, "Konrad Windszus" <konra...@gmx.de> wrote: >> >> >> >> >> >> >It is not (only) about throwing exceptions in case no suitable >> >>adapter >> >> >>is >> >> >> >available. It rather is about the fact, that today the adaptTo >>is a >> >> >> >barrier for all kinds of exceptions. In some cases the adaptation >> >>fails >> >> >> >for a specific reason (one example is Sling Models where >>injection >> >> >>fails, >> >> >> >another one is the issue mentioned in >> >> >> >https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not >> >> >>supporting >> >> >> >primitives)). Both would be valid use cases where the client >>would >> >>be >> >> >> >interested to catch the exception here. >> >> >> > >> >> >> >On 01 Jul 2014, at 10:34, Carsten Ziegeler <cziege...@apache.org> >> >> >>wrote: >> >> >> > >> >> >> >> Adding a new interface would require us to implement it all >>over >> >>the >> >> >> >>place >> >> >> >> and as Felix points out, client code would always need to check >> >> >>whether >> >> >> >>the >> >> >> >> new interface is implemented or not Having to methods, like >> >> >>hasAdapter >> >> >> >>and >> >> >> >> adaptOrThrow does not work very well as between the two calls >> >>things >> >> >> >>might >> >> >> >> have changed already: while hasAdapter returns true, the >> >>underlying >> >> >> >>factory >> >> >> >> gets unregistered before adaptOrThrow is called. >> >> >> >> In many use cases, the current pattern works fine - the client >> >>does >> >> >>not >> >> >> >> care whether an exception is thrown within the adaption - it >>just >> >> >>cares >> >> >> >> whether an object is returned or not. And there are valid use >> >>cases, >> >> >> >>where >> >> >> >> client code does different things whether the adaption works or >> >>not >> >> >> >>(e.g. >> >> >> >> the post servlet checks for adaptTo(Node) and then does >>additional >> >> >> >>things >> >> >> >> if the resource is backed up by a node.) >> >> >> >> >> >> >> >> I see the point that there are also use cases where it would be >> >>fine >> >> >>to >> >> >> >> simpy throw an exception if adaptTo is not successful. This >>would >> >> >>make >> >> >> >>the >> >> >> >> client code easier. However as this most properly is a runtime >> >> >> >>exception, >> >> >> >> client code can today just call a method on the object and end >>up >> >> >>with a >> >> >> >> NPE - having the same result :) >> >> >> >> >> >> >> >> This leaves us with use cases where the client code explicitely >> >> >>wants to >> >> >> >> catch the exception and then do something depending on the >> >>exception. >> >> >> >>Maybe >> >> >> >> we should just add something for this explicit use case >>instead of >> >> >> >>bloating >> >> >> >> the general adaptTo mechanism? >> >> >> >> >> >> >> >> Regards >> >> >> >> Carsten >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> 2014-07-01 9:44 GMT+02:00 Konrad Windszus <konra...@gmx.de>: >> >> >> >> >> >> >> >>> Regarding 1) Having such a Result class would mean that all >> >>consumer >> >> >> >>>would >> >> >> >>> need to unwrap the exception first. So instead of being >>forced of >> >> >> >>> implementing a null-check (as with the old solution) one would >> >>need >> >> >>to >> >> >> >>> implement another check. I want to prevent such a burden to >>the >> >> >> >>>consumers. >> >> >> >>> Regarding 2) Since the client code knows on which object the >> >> >> >>> adaptToOrThrow is called I don¹t get why an instanceof check >>is >> >> >> >>>necessary. >> >> >> >>> Whether this object implements Adaptable2 is known at >> >>compile-time, >> >> >>so >> >> >> >>>you >> >> >> >>> do have the full IDE-support here. >> >> >> >>> Regarding 3) In that case I would no longer allow a null value >> >>to be >> >> >> >>> returned. One drawback is, that all the null checks are no >>longer >> >> >> >>>effective. >> >> >> >>> >> >> >> >>> IMHO solution 2) is the best. At the same time I would >>deprecate >> >>the >> >> >> >>>old >> >> >> >>> Adaptable, because I cannot come up with a real use-case where >> >> >> >>>returning >> >> >> >>> has an advantage. I would rather implement another method >>boolean >> >> >> >>> hasAdapter(Class<AdapterType> type) on the Adaptable2 >>interface. >> >> >> >>> Regards, >> >> >> >>> Konrad >> >> >> >>> >> >> >> >>> On 01 Jul 2014, at 09:07, Felix Meschberger >><fmesc...@adobe.com> >> >> >> wrote: >> >> >> >>> >> >> >> >>>> Hi >> >> >> >>>> >> >> >> >>>> There currently are two issues floating around dealing with >>the >> >> >> >>>>question >> >> >> >>> of returning more information than just null from the >> >> >> >>> Adaptable.adaptTo(Class) method: >> >> >> >>> https://issues.apache.org/jira/browse/SLING-3714 and >> >> >> >>> https://issues.apache.org/jira/browse/SLING-3709. I think >>these >> >> >> >>>requests >> >> >> >>> warrant some discussion on the list. >> >> >> >>>> >> >> >> >>>> Background: adaptTo can be implemented by Adaptable >> >>implementations >> >> >> >>> themselves or, by extending from SlingAdaptable, they may >>defer >> >>to >> >> >>an >> >> >> >>> AdapterMananager. AdapterManager itself is extended by >> >> >>AdapterFactory >> >> >> >>> services. All these interfaces define an adaptTo method. All >> >>these >> >> >> >>>methods >> >> >> >>> return null if adaption is not possible and don't declare or >> >> >>document >> >> >> >>>to >> >> >> >>> throw an exception. >> >> >> >>>> >> >> >> >>>> While not explicitly documented as such, the intention is and >> >>was >> >> >>that >> >> >> >>> adaptTo never throws on the grounds that adaption may fail >>which >> >>is >> >> >> >>> considered a valid result and thus exceptions are not to be >> >>expected >> >> >> >>>and >> >> >> >>> handled. >> >> >> >>>> >> >> >> >>>> Hence all implementations of the methods generally >> >> >> >>> catch-and-log-but-don't-throw. Interestingly >> >>SlingAdaptable.adaptTo >> >> >>and >> >> >> >>> AdapterManagerImpl.getAdapter don't catch ‹ so any >> >>RuntimeException >> >> >> >>>thrown >> >> >> >>> from an AdapterFactory would be forwarded. >> >> >> >>>> >> >> >> >>>> Having said this there are options available: >> >> >> >>>> >> >> >> >>>> (1) Add support for a new Result<?> class. We would probably >> >> >>implement >> >> >> >>> this in the AdapterManager.getAdapter implementation >>explicitly >> >> >> >>>handling >> >> >> >>> this case because it would entail catching the >>adaptTo/getAdapter >> >> >> >>>calls to >> >> >> >>> get the exception (the Result.getError should return Throwable >> >> >> >>>probably not >> >> >> >>> Error) >> >> >> >>>> >> >> >> >>>> Use would be limited to new AdapterFactory implementations >> >>throwing >> >> >> >>> RuntimeExcpetion. For Sling Models this would be the case. >> >> >> >>>> >> >> >> >>>> (2) Add a new adaptToOrThrow method, which is declared to >>throw >> >>a >> >> >> >>> RuntimeException and never return null: Either it can adapt >>or it >> >> >> >>>throws. >> >> >> >>> This would require a new interface Adaptable2 (probably) to >>not >> >> >>break >> >> >> >>> existing Adaptable implementations. The SlingAdaptable base >>class >> >> >>would >> >> >> >>> implement the new method of course, probably something like >>this: >> >> >> >>>> >> >> >> >>>>> SlingAdaptable implements Adaptable2 { >> >> >> >>>>> Š >> >> >> >>>>> public <AdapterType> AdapterType >> >>adaptToOrThrow(Class<AdapterType> >> >> >> >>> type) { >> >> >> >>>>> AdapterType result = this.adaptTo(type); >> >> >> >>>>> if (result != null) { >> >> >> >>>>> return result; >> >> >> >>>>> } >> >> >> >>>>> throw new CannotAdaptException(Š); >> >> >> >>>>> } >> >> >> >>>>> } >> >> >> >>>>> >> >> >> >>>> >> >> >> >>>> Use is problematic because you would have to know whether you >> >>can >> >> >>call >> >> >> >>> the new method: So instead of an null check you now have an >> >> >>instanceof >> >> >> >>> check Š Except for the Resource interface which would be >> >>extended to >> >> >> >>>extend >> >> >> >>> from Adaptable2 as well. >> >> >> >>>> >> >> >> >>>> (3) Document, that Adaptable.adaptTo may throw a >> >>RuntimeException. >> >> >> >>>> >> >> >> >>>> The problem here is, that this may conceptually break >>existing >> >> >>callers >> >> >> >>> of Adaptable.adaptTo which don't expect an exception at all ‹ >> >> >> >>>presumably >> >> >> >>> this is a minor nuisance because technically a >>RuntimeException >> >>may >> >> >> >>>always >> >> >> >>> be thrown. >> >> >> >>>> >> >> >> >>>> Regards >> >> >> >>>> Felix >> >> >> >>> >> >> >> >>> >> >> >> >> >> >> >> >> >> >> >> >> -- >> >> >> >> Carsten Ziegeler >> >> >> >> Adobe Research Switzerland >> >> >> >> cziege...@apache.org >> >> >> > >> >> >> >> >> >> >> >> > >> >> > >> >> >-- >> >> >Carsten Ziegeler >> >> >Adobe Research Switzerland >> >> >cziege...@apache.org >> >> >> >> >> > >> > >> >-- >> >Carsten Ziegeler >> >Adobe Research Switzerland >> >cziege...@apache.org >> >> > > >-- >Carsten Ziegeler >Adobe Research Switzerland >cziege...@apache.org
