Re: onException is ignored

2015-01-14 Thread Willem Jiang
I just saw the org.slf4j.helpers.MarkerIgnoringBase is use to write the warning 
and error message. I guess it may relate to your log configuration.

--  
Willem Jiang

Red Hat, Inc.
Web: http://www.redhat.com
Blog: http://willemjiang.blogspot.com (English)
http://jnn.iteye.com (Chinese)
Twitter: willemjiang  
Weibo: 姜宁willem



On January 12, 2015 at 10:41:43 PM, James Green (james.mk.gr...@gmail.com) 
wrote:
> Mea culpa. Who knew AccountNumberFoundException was part of
> javax.security..! Fixed the import, the onException now executes.
>  
> However, the .log line:
> .log(LoggingLevel.ERROR, "com.foo.server.ngw", "Account Not
> Found. Message discarded.").stop();
>  
> Actually writes the line:
>  
> 14:22:03,993 ERROR org.slf4j.helpers.MarkerIgnoringBase:145 error() -
> Account Not Found. Message discarded.
>  
> So what was the point of supplying a logName?
>  
>  
> On 12 January 2015 at 12:05, James Green wrote:
>  
> > As suggested:
> >
> > errorHandler(transactionErrorHandler().maximumRedeliveries(3));
> > onException(AccountNotFoundException.class)
> > .log(LoggingLevel.ERROR, "com.foo.server.ngw", "Account
> > Not Found. Message discarded.").stop();
> >
> >
> > from(source())
> > .transacted()
> > .unmarshal(jacksonUnmarshall)
> > .process(router)
> > .recipientList(simple("${body.media}"));
> >
> > I can see Camel attempt redelivery three times as a result of catching
> > AccountNotFoundException. After the fourth and final attempt fails I can't
> > actually see much different to that logged for the previous three attempts.
> > I've kept the reporting down to what I hope is pertinent below:
> >
> > 10:32:23,885 ERROR org.slf4j.helpers.MarkerIgnoringBase:161 error() -
> > Failed delivery for (MessageId:
> > queue_inbound_ID_JGREENWIN7-54570-142105884-3_1_1_1_3 on ExchangeId:  
> > ID-JGREENWIN7-54739-1421058738390-0-6). Exhausted after delivery attempt: 4 
> >  
> > caught: com.foo.server.ngw.router.AccountNotFoundException: Account not
> > found
> > ...
> > 10:32:23,932 WARN org.apache.camel.spring.spi.TransactionErrorHandler:287  
> > logTransactionRollback() - Transaction rollback (0x2017df6a)
> > redelivered(true) for (MessageId:
> > ID:JGREENWIN7-54570-142105884-3:1:1:1:3 on ExchangeId:
> > ID-JGREENWIN7-54739-1421058738390-0-7) caught:
> > com.foo.server.ngw.router.AccountNotFoundException: Account not found
> > 10:32:23,932 WARN org.slf4j.helpers.MarkerIgnoringBase:136 warn() -
> > Execution of JMS message listener failed. Caused by:
> > [org.apache.camel.RuntimeCamelException -
> > com.foo.server.ngw.router.AccountNotFoundException: Account not found]
> > org.apache.camel.RuntimeCamelException:
> > com.foo.server.ngw.router.AccountNotFoundException: Account not found
> > at
> > org.apache.camel.util.ObjectHelper.wrapRuntimeCamelException(ObjectHelper.java:1364)
> >   
> > at
> > org.apache.camel.spring.spi.TransactionErrorHandler$1.doInTransactionWithoutResult(TransactionErrorHandler.java:188)
> >   
> > at
> > org.springframework.transaction.support.TransactionCallbackWithoutResult.doInTransaction(TransactionCallbackWithoutResult.java:34)
> >   
> > at
> > org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:133)
> >   
> > at
> > org.apache.camel.spring.spi.TransactionErrorHandler.doInTransactionTemplate(TransactionErrorHandler.java:174)
> >   
> > at
> > org.apache.camel.spring.spi.TransactionErrorHandler.processInTransaction(TransactionErrorHandler.java:134)
> >   
> > at
> > org.apache.camel.spring.spi.TransactionErrorHandler.process(TransactionErrorHandler.java:103)
> >   
> > at
> > org.apache.camel.spring.spi.TransactionErrorHandler.process(TransactionErrorHandler.java:112)
> >   
> > at
> > org.apache.camel.processor.CamelInternalProcessor.process(CamelInternalProcessor.java:191)
> >   
> > at
> > org.apache.camel.processor.CamelInternalProcessor.process(CamelInternalProcessor.java:191)
> >   
> > at
> > org.apache.camel.util.AsyncProcessorHelper.process(AsyncProcessorHelper.java:105)
> >   
> > at
> > org.apache.camel.processor.DelegateAsyncProcessor.process(DelegateAsyncProcessor.java:87)
> >   
> > at
> > org.apache.camel.component.jms.EndpointMessageListener.onMessage(EndpointMessageListener.java:103)
> >   
> > at
> > org.springframework.jms.listener.AbstractMessageListenerContainer.doInvokeListener(AbstractMessageListenerContainer.java:562)
> >   
> > at
> > org.springframework.jms.listener.AbstractMessageListenerContainer.invokeListener(AbstractMessageListenerContainer.java:500)
> >   
> > at
> > org.springframework.jms.listener.AbstractMessageListenerContainer.doExecuteListener(AbstractMessageListenerContainer.java:468)
> >   
> > at
> > org.springframework.jms.listener.AbstractPollingMessageListenerContainer.doReceiveAndExecute(AbstractPollingMessageListenerContainer.java:325)
> >   
> > at
> > org.springframework.jms.listener.AbstractPollingMessageListenerContainer.receiveAndExecute(AbstractPollingMessageListenerContainer.

Re: onException is ignored

2015-01-12 Thread James Green
Mea culpa. Who knew AccountNumberFoundException was part of
javax.security..! Fixed the import, the onException now executes.

However, the .log line:
.log(LoggingLevel.ERROR, "com.foo.server.ngw", "Account Not
Found. Message discarded.").stop();

Actually writes the line:

14:22:03,993 ERROR org.slf4j.helpers.MarkerIgnoringBase:145 error() -
Account Not Found. Message discarded.

So what was the point of supplying a logName?


On 12 January 2015 at 12:05, James Green  wrote:

> As suggested:
>
> errorHandler(transactionErrorHandler().maximumRedeliveries(3));
> onException(AccountNotFoundException.class)
> .log(LoggingLevel.ERROR, "com.foo.server.ngw", "Account
> Not Found. Message discarded.").stop();
>
>
> from(source())
> .transacted()
> .unmarshal(jacksonUnmarshall)
> .process(router)
> .recipientList(simple("${body.media}"));
>
> I can see Camel attempt redelivery three times as a result of catching
> AccountNotFoundException. After the fourth and final attempt fails I can't
> actually see much different to that logged for the previous three attempts.
> I've kept the reporting down to what I hope is pertinent below:
>
> 10:32:23,885 ERROR org.slf4j.helpers.MarkerIgnoringBase:161 error() -
> Failed delivery for (MessageId:
> queue_inbound_ID_JGREENWIN7-54570-142105884-3_1_1_1_3 on ExchangeId:
> ID-JGREENWIN7-54739-1421058738390-0-6). Exhausted after delivery attempt: 4
> caught: com.foo.server.ngw.router.AccountNotFoundException: Account not
> found
> ...
> 10:32:23,932  WARN org.apache.camel.spring.spi.TransactionErrorHandler:287
> logTransactionRollback() - Transaction rollback (0x2017df6a)
> redelivered(true) for (MessageId:
> ID:JGREENWIN7-54570-142105884-3:1:1:1:3 on ExchangeId:
> ID-JGREENWIN7-54739-1421058738390-0-7) caught:
> com.foo.server.ngw.router.AccountNotFoundException: Account not found
> 10:32:23,932  WARN org.slf4j.helpers.MarkerIgnoringBase:136 warn() -
> Execution of JMS message listener failed. Caused by:
> [org.apache.camel.RuntimeCamelException -
> com.foo.server.ngw.router.AccountNotFoundException: Account not found]
> org.apache.camel.RuntimeCamelException:
> com.foo.server.ngw.router.AccountNotFoundException: Account not found
> at
> org.apache.camel.util.ObjectHelper.wrapRuntimeCamelException(ObjectHelper.java:1364)
> at
> org.apache.camel.spring.spi.TransactionErrorHandler$1.doInTransactionWithoutResult(TransactionErrorHandler.java:188)
> at
> org.springframework.transaction.support.TransactionCallbackWithoutResult.doInTransaction(TransactionCallbackWithoutResult.java:34)
> at
> org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:133)
> at
> org.apache.camel.spring.spi.TransactionErrorHandler.doInTransactionTemplate(TransactionErrorHandler.java:174)
> at
> org.apache.camel.spring.spi.TransactionErrorHandler.processInTransaction(TransactionErrorHandler.java:134)
> at
> org.apache.camel.spring.spi.TransactionErrorHandler.process(TransactionErrorHandler.java:103)
> at
> org.apache.camel.spring.spi.TransactionErrorHandler.process(TransactionErrorHandler.java:112)
> at
> org.apache.camel.processor.CamelInternalProcessor.process(CamelInternalProcessor.java:191)
> at
> org.apache.camel.processor.CamelInternalProcessor.process(CamelInternalProcessor.java:191)
> at
> org.apache.camel.util.AsyncProcessorHelper.process(AsyncProcessorHelper.java:105)
> at
> org.apache.camel.processor.DelegateAsyncProcessor.process(DelegateAsyncProcessor.java:87)
> at
> org.apache.camel.component.jms.EndpointMessageListener.onMessage(EndpointMessageListener.java:103)
> at
> org.springframework.jms.listener.AbstractMessageListenerContainer.doInvokeListener(AbstractMessageListenerContainer.java:562)
> at
> org.springframework.jms.listener.AbstractMessageListenerContainer.invokeListener(AbstractMessageListenerContainer.java:500)
> at
> org.springframework.jms.listener.AbstractMessageListenerContainer.doExecuteListener(AbstractMessageListenerContainer.java:468)
> at
> org.springframework.jms.listener.AbstractPollingMessageListenerContainer.doReceiveAndExecute(AbstractPollingMessageListenerContainer.java:325)
> at
> org.springframework.jms.listener.AbstractPollingMessageListenerContainer.receiveAndExecute(AbstractPollingMessageListenerContainer.java:243)
> at
> org.springframework.jms.listener.DefaultMessageListenerContainer$AsyncMessageListenerInvoker.invokeListener(DefaultMessageListenerContainer.java:1101)
> at
> org.springframework.jms.listener.DefaultMessageListenerContainer$AsyncMessageListenerInvoker.executeOngoingLoop(DefaultMessageListenerContainer.java:1093)
> at
> org.springframework.jms.listener.DefaultMessageListenerContainer$AsyncMessageListenerInvoker.run(DefaultMessageListenerContainer.java:990)
> at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
> at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown

Re: onException is ignored

2015-01-12 Thread James Green
As suggested:

errorHandler(transactionErrorHandler().maximumRedeliveries(3));
onException(AccountNotFoundException.class)
.log(LoggingLevel.ERROR, "com.foo.server.ngw", "Account Not
Found. Message discarded.").stop();


from(source())
.transacted()
.unmarshal(jacksonUnmarshall)
.process(router)
.recipientList(simple("${body.media}"));

I can see Camel attempt redelivery three times as a result of catching
AccountNotFoundException. After the fourth and final attempt fails I can't
actually see much different to that logged for the previous three attempts.
I've kept the reporting down to what I hope is pertinent below:

10:32:23,885 ERROR org.slf4j.helpers.MarkerIgnoringBase:161 error() -
Failed delivery for (MessageId:
queue_inbound_ID_JGREENWIN7-54570-142105884-3_1_1_1_3 on ExchangeId:
ID-JGREENWIN7-54739-1421058738390-0-6). Exhausted after delivery attempt: 4
caught: com.foo.server.ngw.router.AccountNotFoundException: Account not
found
...
10:32:23,932  WARN org.apache.camel.spring.spi.TransactionErrorHandler:287
logTransactionRollback() - Transaction rollback (0x2017df6a)
redelivered(true) for (MessageId:
ID:JGREENWIN7-54570-142105884-3:1:1:1:3 on ExchangeId:
ID-JGREENWIN7-54739-1421058738390-0-7) caught:
com.foo.server.ngw.router.AccountNotFoundException: Account not found
10:32:23,932  WARN org.slf4j.helpers.MarkerIgnoringBase:136 warn() -
Execution of JMS message listener failed. Caused by:
[org.apache.camel.RuntimeCamelException -
com.foo.server.ngw.router.AccountNotFoundException: Account not found]
org.apache.camel.RuntimeCamelException:
com.foo.server.ngw.router.AccountNotFoundException: Account not found
at
org.apache.camel.util.ObjectHelper.wrapRuntimeCamelException(ObjectHelper.java:1364)
at
org.apache.camel.spring.spi.TransactionErrorHandler$1.doInTransactionWithoutResult(TransactionErrorHandler.java:188)
at
org.springframework.transaction.support.TransactionCallbackWithoutResult.doInTransaction(TransactionCallbackWithoutResult.java:34)
at
org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:133)
at
org.apache.camel.spring.spi.TransactionErrorHandler.doInTransactionTemplate(TransactionErrorHandler.java:174)
at
org.apache.camel.spring.spi.TransactionErrorHandler.processInTransaction(TransactionErrorHandler.java:134)
at
org.apache.camel.spring.spi.TransactionErrorHandler.process(TransactionErrorHandler.java:103)
at
org.apache.camel.spring.spi.TransactionErrorHandler.process(TransactionErrorHandler.java:112)
at
org.apache.camel.processor.CamelInternalProcessor.process(CamelInternalProcessor.java:191)
at
org.apache.camel.processor.CamelInternalProcessor.process(CamelInternalProcessor.java:191)
at
org.apache.camel.util.AsyncProcessorHelper.process(AsyncProcessorHelper.java:105)
at
org.apache.camel.processor.DelegateAsyncProcessor.process(DelegateAsyncProcessor.java:87)
at
org.apache.camel.component.jms.EndpointMessageListener.onMessage(EndpointMessageListener.java:103)
at
org.springframework.jms.listener.AbstractMessageListenerContainer.doInvokeListener(AbstractMessageListenerContainer.java:562)
at
org.springframework.jms.listener.AbstractMessageListenerContainer.invokeListener(AbstractMessageListenerContainer.java:500)
at
org.springframework.jms.listener.AbstractMessageListenerContainer.doExecuteListener(AbstractMessageListenerContainer.java:468)
at
org.springframework.jms.listener.AbstractPollingMessageListenerContainer.doReceiveAndExecute(AbstractPollingMessageListenerContainer.java:325)
at
org.springframework.jms.listener.AbstractPollingMessageListenerContainer.receiveAndExecute(AbstractPollingMessageListenerContainer.java:243)
at
org.springframework.jms.listener.DefaultMessageListenerContainer$AsyncMessageListenerInvoker.invokeListener(DefaultMessageListenerContainer.java:1101)
at
org.springframework.jms.listener.DefaultMessageListenerContainer$AsyncMessageListenerInvoker.executeOngoingLoop(DefaultMessageListenerContainer.java:1093)
at
org.springframework.jms.listener.DefaultMessageListenerContainer$AsyncMessageListenerInvoker.run(DefaultMessageListenerContainer.java:990)
at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
at java.lang.Thread.run(Unknown Source)
Caused by: com.foo.server.ngw.router.AccountNotFoundException: Account not
found
at
com.foo.server.ngw.router.MessageToSendFactory.create(MessageToSendFactory.java:29)
at com.foo.server.ngw.router.MediaRouter.process(MediaRouter.java:31)
at
org.apache.camel.processor.DelegateSyncProcessor.process(DelegateSyncProcessor.java:63)
at
org.apache.camel.management.InstrumentationProcessor.process(InstrumentationProcessor.java:72)
at
org.apache.camel.processor.RedeliveryErrorHandler.process(RedeliveryErrorHandler.java:416)
at
org.apache.camel.spring.spi.TransactionErrorHandler.processByErrorHandler(Transactio

Re: onException is ignored

2015-01-12 Thread Claus Ibsen
Hi

No the onException should be triggered even for TX.

You may try to configure onException after errorHandler.


On Mon, Jan 12, 2015 at 11:02 AM, James Green  wrote:
> So when using onException it's really important not to use a component that
> is already partaking in the transaction being rolled back? I get that now,
> but it's not immediately obvious.
>
> Does this explain why the log()ged message, "Account Not Found. Message
> discarded." does not appear in the console though?
>
> On 10 January 2015 at 07:23, Willem Jiang  wrote:
>
>> If the Exception is thrown from the source() endpoint, the onException
>> error handler won’t work as you expected.
>>
>> --
>> Willem Jiang
>>
>> Red Hat, Inc.
>> Web: http://www.redhat.com
>> Blog: http://willemjiang.blogspot.com (English)
>> http://jnn.iteye.com (Chinese)
>> Twitter: willemjiang
>> Weibo: 姜宁willem
>>
>>
>>
>> On January 10, 2015 at 1:38:52 AM, James Green (james.mk.gr...@gmail.com)
>> wrote:
>> > Project is Spring based with Camel 2.14 and the following configuration:
>> >
>> > onException(AccountNotFoundException.class)
>> > .log("Account Not Found. Message
>> > discarded.").to("jms:queue:RouterAccountNotFound").stop();
>> >
>> > errorHandler(transactionErrorHandler().maximumRedeliveries(3));
>> >
>> > from(source())
>> > .transacted()
>> > .unmarshal(jacksonUnmarshall)
>> > .process(router)
>> > .recipientList(simple("${body.media}"));
>> >
>> > We have a JMS connection and a JPA connection wrapped in a Atomikos
>> > transaction manager (a XA transaction manager I understand to be
>> required).
>> > The above errorHandler triggers if the router throws an Exception, but
>> the
>> > onException statement does not fire despite the logs recording the
>> > AccountNotFoundException being caught by Camel.
>> >
>> > Consequently the log()ing and the RouterAccountNotFound queue do not fire
>> > at all.
>> >
>> > The documentation suggests onException should be usable at this point.
>> Can
>> > anyone suggest why the above only partly works?
>> >
>> > Thanks,
>> >
>> > James
>> >
>>
>>



-- 
Claus Ibsen
-
Red Hat, Inc.
Email: cib...@redhat.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen
hawtio: http://hawt.io/
fabric8: http://fabric8.io/


Re: onException is ignored

2015-01-12 Thread James Green
So when using onException it's really important not to use a component that
is already partaking in the transaction being rolled back? I get that now,
but it's not immediately obvious.

Does this explain why the log()ged message, "Account Not Found. Message
discarded." does not appear in the console though?

On 10 January 2015 at 07:23, Willem Jiang  wrote:

> If the Exception is thrown from the source() endpoint, the onException
> error handler won’t work as you expected.
>
> --
> Willem Jiang
>
> Red Hat, Inc.
> Web: http://www.redhat.com
> Blog: http://willemjiang.blogspot.com (English)
> http://jnn.iteye.com (Chinese)
> Twitter: willemjiang
> Weibo: 姜宁willem
>
>
>
> On January 10, 2015 at 1:38:52 AM, James Green (james.mk.gr...@gmail.com)
> wrote:
> > Project is Spring based with Camel 2.14 and the following configuration:
> >
> > onException(AccountNotFoundException.class)
> > .log("Account Not Found. Message
> > discarded.").to("jms:queue:RouterAccountNotFound").stop();
> >
> > errorHandler(transactionErrorHandler().maximumRedeliveries(3));
> >
> > from(source())
> > .transacted()
> > .unmarshal(jacksonUnmarshall)
> > .process(router)
> > .recipientList(simple("${body.media}"));
> >
> > We have a JMS connection and a JPA connection wrapped in a Atomikos
> > transaction manager (a XA transaction manager I understand to be
> required).
> > The above errorHandler triggers if the router throws an Exception, but
> the
> > onException statement does not fire despite the logs recording the
> > AccountNotFoundException being caught by Camel.
> >
> > Consequently the log()ing and the RouterAccountNotFound queue do not fire
> > at all.
> >
> > The documentation suggests onException should be usable at this point.
> Can
> > anyone suggest why the above only partly works?
> >
> > Thanks,
> >
> > James
> >
>
>


Re: onException is ignored

2015-01-09 Thread Willem Jiang
If the Exception is thrown from the source() endpoint, the onException error 
handler won’t work as you expected.

--  
Willem Jiang

Red Hat, Inc.
Web: http://www.redhat.com
Blog: http://willemjiang.blogspot.com (English)
http://jnn.iteye.com (Chinese)
Twitter: willemjiang  
Weibo: 姜宁willem



On January 10, 2015 at 1:38:52 AM, James Green (james.mk.gr...@gmail.com) wrote:
> Project is Spring based with Camel 2.14 and the following configuration:
>  
> onException(AccountNotFoundException.class)
> .log("Account Not Found. Message
> discarded.").to("jms:queue:RouterAccountNotFound").stop();
>  
> errorHandler(transactionErrorHandler().maximumRedeliveries(3));
>  
> from(source())
> .transacted()
> .unmarshal(jacksonUnmarshall)
> .process(router)
> .recipientList(simple("${body.media}"));
>  
> We have a JMS connection and a JPA connection wrapped in a Atomikos
> transaction manager (a XA transaction manager I understand to be required).
> The above errorHandler triggers if the router throws an Exception, but the
> onException statement does not fire despite the logs recording the
> AccountNotFoundException being caught by Camel.
>  
> Consequently the log()ing and the RouterAccountNotFound queue do not fire
> at all.
>  
> The documentation suggests onException should be usable at this point. Can
> anyone suggest why the above only partly works?
>  
> Thanks,
>  
> James
>  



Re: onException is ignored

2015-01-09 Thread Claus Ibsen
Since you use transactions then sending a message to a JMS queue is
also rolled back, and hence the onException "appears" to not send the
message to the account not found queue. But its because the tx is
rolled back.

You can define a 2nd jms component, eg using a different name than
jms, that does not become part of the same tx, and use that in the
onException.



On Fri, Jan 9, 2015 at 6:38 PM, James Green  wrote:
> Project is Spring based with Camel 2.14 and the following configuration:
>
> onException(AccountNotFoundException.class)
> .log("Account Not Found. Message
> discarded.").to("jms:queue:RouterAccountNotFound").stop();
>
> errorHandler(transactionErrorHandler().maximumRedeliveries(3));
>
> from(source())
> .transacted()
> .unmarshal(jacksonUnmarshall)
> .process(router)
> .recipientList(simple("${body.media}"));
>
> We have a JMS connection and a JPA connection wrapped in a Atomikos
> transaction manager (a XA transaction manager I understand to be required).
> The above errorHandler triggers if the router throws an Exception, but the
> onException statement does not fire despite the logs recording the
> AccountNotFoundException being caught by Camel.
>
> Consequently the log()ing and the RouterAccountNotFound queue do not fire
> at all.
>
> The documentation suggests onException should be usable at this point. Can
> anyone suggest why the above only partly works?
>
> Thanks,
>
> James



-- 
Claus Ibsen
-
Red Hat, Inc.
Email: cib...@redhat.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen
hawtio: http://hawt.io/
fabric8: http://fabric8.io/


onException is ignored

2015-01-09 Thread James Green
Project is Spring based with Camel 2.14 and the following configuration:

onException(AccountNotFoundException.class)
.log("Account Not Found. Message
discarded.").to("jms:queue:RouterAccountNotFound").stop();

errorHandler(transactionErrorHandler().maximumRedeliveries(3));

from(source())
.transacted()
.unmarshal(jacksonUnmarshall)
.process(router)
.recipientList(simple("${body.media}"));

We have a JMS connection and a JPA connection wrapped in a Atomikos
transaction manager (a XA transaction manager I understand to be required).
The above errorHandler triggers if the router throws an Exception, but the
onException statement does not fire despite the logs recording the
AccountNotFoundException being caught by Camel.

Consequently the log()ing and the RouterAccountNotFound queue do not fire
at all.

The documentation suggests onException should be usable at this point. Can
anyone suggest why the above only partly works?

Thanks,

James